docs(x/cosmwasmpool): code id whitelist via gov props

[p0mvn]

Closes: #XXX

What is the purpose of the change

Adding documentation on how I'm planning to handle code id management via gov props.

We briefly discussed this approach via comments in: #4810

Would like to converge on this before I spend time implementing.

Testing and Verifying

This change is a trivial rework / code cleanup without any test coverage.

Documentation and Release Note

• Does this pull request introduce a new feature or user-facing behavior changes?
• Changelog entry added to Unreleased section of CHANGELOG.md?

Where is the change documented?

• Specification (x/{module}/README.md)
• Osmosis documentation site
• Code comments?
• N/A

[iboss-ptk]

except from x/cosmwasm the rest looks good!

[nicolaslara]

Suggested change

	2. Store code and migrate a specific code id to a new code id.
	2. Store code and migrate a contract to the newly uploaded code id.

Question: what if we want to migrate to another code id that has already been uploaded and whitelisted? If that is allowed, isn't this proposal redundant, as it's a combination of the one above and a migrate message?

[p0mvn]

I think we should require going through this message if the code id was uploaded separately.

The reason is that code upload is allowed to a certain set of addresses. I don't know who is the owner, and I don't think these addresses should be able to upload contract code that is meant to be used as a pool model without going through a governance proposal that is meant to add the code id to the whitelist.

So I think people can still upload the code separately and reuse another code id. However, they should go through a gov prop for that code id to be added to the whitelist.

If there is no such governance-approved whitelist for pool code ids, one of the addresses below could, in theory, upload a malicious contract and migrate a pool to it, causing a loss of funds.

osmosisd q wasm params
code_upload_access:
  address: ""
  addresses:
  - osmo1cd4nn8yzdrrsfqsmmvaafq8r03xn38qgqt8fzh
  - osmo1wl59k23zngj34l7d42y9yltask7rjlnxgccawc7ltrknp6n52fps94qsjd
  - osmo19vxp8vq8qm368dr026qxh8v82satwaf79y235lfv6wmgpwxx8dtskedaku
  - osmo1e0x2hnhhwyek7eq3kcxu2x6pt77wdnwz0lutz9fespdr9utq963qr0y5p5
  - osmo14n3a65fnqz9jve85l23al6m3pjugf0atvrfqh5
  - osmo15wna5dwylkuzvljsudyn6zfsd4zl0rkg5ge888mzk4vtnjpp0z5q4e9w58
  - osmo1r02tlyyaqs6tmrfa4jf37t7ewuxr57qp8ghzly
  permission: AnyOfAddresses
instantiate_default_permission: Everybody

[nicolaslara]

I understand that we should require migrating to a code id that has been uploaded by that method. My reasoning is:

• There's code_id 1 for a pool type (say, transmutter)
• 10 transmutter pools get created
• Someone creates an improved version of that code and uploads it and migrates their contract using this method.
• Now we have code_id 2 and one pool using that code. The other 9 use code_id 1
• If they want to migrate to the new code, they shouldn't need to upload code ids 3..11, they should just be able to use code_id 2 as it has already been whitelisted for pools.

Does that make sense? or is the idea that uploading code_id 2 in the example will migrate all pools using code_id 1?

[p0mvn]

I understand. Good point. The way I've been thinking about it is that all pool instantiations of the same contract should be associated with the same code id..

Similar to how there is one kind of balancer, stable swap and CL pool. There aren't many different versions of these pool models. If we upgrade one, we upgrade all.

For example, assuming transmuter, I would not want to have different iterations available. Pool models and contracts should be as stable as possible. So if there is an upgrade to transmuter, all pools should be upgraded and migrated at once. I wouldn't want some pools to be left behind on the old code id.

If someone needs a customized implementation of the transmuter, that's a different story. In that case, I think this customized version should be re-uploaded separately and follow a different migration path for all pool instances of that customized transmuter type.

However, putting in more thought into this now, I think migrating a bunch of pools at once has a risk of being resource intensive and not scalable. It is not a problem today since we have like 1000 pools in total. However, thinking long-term I'm not sure if this is ideal.

I was also thinking about a spam vector but I don't think that's a concern since pool creation fee is charged and this message is governance-gated.

I will think about this more and maybe start implementing to have a better idea of the trade-offs. If you have any thoughts in the meantime, please lmk

[p0mvn]

Conclusion

Upon more consideration, I think there should be the following migration proposal:

1. MigrateMultipleCWPoolsByPoolID

Migrates all given cw pool contracts specified by their IDs. It has two options to perform the migration.

If the codeID is non-zero, it will migrate the pool contracts to a given codeID assuming that it has already been uploaded. uploadByteCode must be empty in such a case. Fails if codeID does not exist. Fails if uploadByteCode is not empty.

If the codeID is zero, it will upload the given uploadByteCode and use the new resulting code id to migrate the pool to. Errors if uploadByteCode is empty or invalid.

In both cases, if one of the pools specified by the given poolID does not exist, the proposal fails.

The reason for having poolIDs be a slice of ids is to account for the potential need for emergency migration of all old code ids to new code ids, or simply having the flexibility of migrating multiple older pool contracts to a new one at once when there is a release.

poolDs must be at the most size of 20. The proposal fails if more. Note that 20 was chosen arbitrarily to have a constant bound on the number of pools migrated at once. This size will be configured by a module parameter so it can be changed by a constant.

• Inputs
  • poolIDs - []uint64
  • codeID - uint64
  • uploadByteCode - []byte

Analysis of The Approach

• Pros

  • The flexibility is maximized as each pool id can be migrated individually either by uploading a new contract code or migrating to a pre-added one.
  • There is still an ability to migrate all pools belonging to a list of code ids. The max number of pools migrated at once is bounded by a constant parameter.

• Cons

  • If there are multiple iterations of the same type, It might become cumbersome to keep track of which pool is on which code id, why one is migrated and the other one is not
  • Some conditionality with proposal parameters. For example, if codeId is zero, byte code must be empty.

Overall, I think pros outweigh the cons, and this is the best approach out of the other alternatives I considered.

Let me know if you agree on the approach, and I will update the spec. In the meantime, I will be prototyping this in #5376

[nicolaslara]

In both cases, if one of the pools is specified by the given poolID, the proposal fails.

Not sure what you mean by this, but generally the approach sounds good.

I don't see mention of checking that the codeID is in the whitelist for pool code ids, but I assime that's implied.

20 pools sounds low, but I don't think that matters as after unforking the sdk we can make proposals with several messages in them

[p0mvn]

Corrected and fixed to:

In both cases, if one of the pools specified by the given `poolID` does not exist, the proposal fails.

Thanks for the catch.

20 pools sounds low, but I don't think that matters as after unforking the sdk we can make proposals with several messages in them

I'm open to a higher number this is chosen since we have 1K pools in total right now and I just want some constant bound. Agreed that this problem also goes away soon.

[p0mvn]

All comments are addressed. Looking for re-reviews please