New instructions for 1-proposal SNS flow

[aterga]

This PR changes the instructions in README.md to document the new, 1-proposal SNS flow. The legacy flow will still be available to users who follow the instructions from here.

To simulate the minimal number of contributors required for completing the swap, run

./participate_sns_swap.sh \
    100 `# one hundred swap participants` \
    10000 `# each contributes 10_000 ICP (i.e., the Maximum Commitment)`

(these are the default parameters)

[mraszyk]

There are still occurrences of dfx canister --network "${NETWORK}" call sns_governance and similarly for other SNS canisters. This won't work unless we add the SNS canister IDs to the JSON files used by DFX (sns_canister_ids.json is not enough).

[mraszyk]

The legacy flow will still be available to users who follow the instructions from here.

Shouldn't we recommend using that other v1-legacy branch also for running the scripts (now it sounds like you can use the scripts from this branch). And maybe also remark that in the README.md on this branch?

[mraszyk]

To simulate the minimal number of contributors required for completing the swap, run

This note from the PR's description refers to the default configuration (I suppose) - shouldn't this be made more explicit?

[aterga]

@mraszyk

There are still occurrences of dfx canister --network "${NETWORK}" call sns_governance and similarly for other SNS canisters. This won't work unless we add the SNS canister IDs to the JSON files used by DFX (sns_canister_ids.json is not enough).

Good catch. I would suggest that we pass explicit arguments to DFX in these cases. If you agree, I'll clean up to make sure we're consistent in this respect.

[aterga]

@mraszyk

The legacy flow will still be available to users who follow the instructions from here.

Shouldn't we recommend using that other v1-legacy branch also for running the scripts (now it sounds like you can use the scripts from this branch). And maybe also remark that in the README.md on this branch?

The current idea is to not mention the legacy flow in this document at all.

Which exact scripts would not work with the new solution?

Generally, I suggest that from now on we only support small set of well-defined flows in this repo. I don't want us to spend too much time trying to create fully modular scripts, only to realize that they are never going to be truly modular, and that having a munch of scripts without telling how exactly they are supposed to be used will lead to a maintenance nightmare for us.

If this doesn't sound right to you, please reach out to discuss this 1:1!

[mraszyk]

The current idea is to not mention the legacy flow in this document at all.

You're mentioning it in the PR description. If that's not supposed to be read, then maybe we should drop it from there. And then if the "legacy" branch is effectively hidden, what's the point of having it? Is it for expert users only?

[mraszyk]

Generally, I suggest that from now on we only support small set of well-defined flows in this repo.

This makes sense: the question is how we arrive at this state. My proposal was to use the "legacy" branch for the "legacy" flow as the CI confirmed that it supports the "legacy" flow. On this branch, the CI is updated to confirm that it works for the "new" flow. If we want to support both flows on the "new" branch, we should also add CI to the "new" branch confirming that the "legacy" flow works on the "new" branch. Does that make sense?

[aterga]

This makes sense: the question is how we arrive at this state. My proposal was to use the "legacy" branch for the "legacy" flow as the CI confirmed that it supports the "legacy" flow. On this branch, the CI is updated to confirm that it works for the "new" flow.

Agreed.

If we want to support both flows on the "new" branch, we should also add CI to the "new" branch confirming that the "legacy" flow works on the "new" branch.

This would make sense in general, but our plan is not to advertise the legacy flow at all, since it will soon be entirely deprecated, and since it is only needed for the few projects that have already started testing with the legacy flow before the 1-prop flow will have been released.

So, the only reason the legacy flow is mentioned in the description of this PR is because the reviewers might have the question, i.e., what should we provide to the above-mentioned category of users (since this PR is deprecating the legacy flow in sns-testing).

Please let me know if this doesn't make sense, @mraszyk

[mraszyk]

The link from the PR description "This PR should be merged after the NNS FE dapp's RC is released via proposal." is dead.

[mraszyk]

the only reason the legacy flow is mentioned in the description of this PR is because the reviewers might have the question, i.e., what should we provide to the above-mentioned category of users (since this PR is deprecating the legacy flow in sns-testing).

If it's a small set of users relying on the legacy flow, why would they use the latest master after merging this MR instead of using a commit to sns-testing master at which the legacy flow properly works (and the CI passes)? In other words, why would we tell those (few) users to use the latest master and a separate document on a branch to tell them how to use the new code for the old flow?

[aterga]

In other words, why would we tell those (few) users to use the latest master and a separate document on a branch to tell them how to use the new code for the old flow?

The current idea is to have the old code with the old flow and the new code with the new flow, that's it.

We don't need a branch that supports both flows.