Investigate how best to support remote contract instantiation

[0xekez]

We should make instantiating contracts and getting the address of the instantiated contract simple.

Currently, in proxy, we parse the data returned from execution:

polytone/contracts/proxy/src/contract.rs

Lines 72 to 75 in ae74667

	collector[msg.id as usize] = match res.data {
	Some(data) => parse_execute_response_data(&data.0)?.data,
	None => None,
	};

I think that if we did not do that parsing, the receiver of the callback may be able to call parse_reply_instantiate_data on the returned data and get the address the same way they normally would in a reply. We should verify this, and consider moving to that system if no better way to instantiate contracts is revealed.

[0xekez]

I think this is best done as part of the handshake process where the voice chain sends over the instantiate2 arguments needed.

[Art3miX]

I think theres a bigger question hiding here with supported features, and you touched it in the handshake spec briefly: https://gist.github.com/0xekez/ea27a25d9de566a579ade4e436f89ff7#try

if we take instantiate2 for example, the note contract doesn't need any special treatment, because we can just create custom structs and send it over to the other chain.
But for the voice to execute instatiate2 the chain needs to support it, because if it doesn't, it will error out.

I think we need to discuss how we handle supported features:

1. Do we need to follow SDK versions on the voice? that way if a feature is supported on the sdk, chains will be able to easily pick the voice they need based on the sdk version.
   Right now instantiate2 is the best feature to test it with, as a lot of chains doesn't support it yet but some does, and we can easily test out how a new feature can be worked out. (make polytone that works only with instantiate1 and see what steps needs to be done to support instantiate2

2. Migrations are gonna be big, we gonna have tons of proxy contracts that will need to be migrated to work with new/better features, I think best course of action with this is to create a UpdateAccount logic where it automatically checks if migration is needed to the account (proxy, based on the voice version) and do the migration if it does needed.
   This will ensure only active accounts are doing migrations.

3. Account timeouts? I think the general idea is that once an account was created, it should stay "open" forever (same like creating a wallet), but we are talking about contracts here, and one simple question that pops to my mind is what happens if account isn't being used for lets say 5 years, and then we need to migrate from v1 to v50.
   Proxy is a simple dispatcher contract and probably gonna stay that way, so this is probably fine, just feels like its important questions we should include in a doc so we won't forget about it in the future.

4. Lets have this case for example: in 1 year, juno wants have some new feature that is only possible for juno, so they update their voice to include that feature.
   In order for a note on stargaze to recognize it, it will need to be updated to include this feature, it will need to do 2 things:
   a. Update handshake to include it as supported
   b. The note will need to have a specific struct for those features, of course we can also accept a generic binary and pass it over, but thats a very bad UX.
   Right now note only accept CosmosMsgs but thats on the note chain, the voice chain can accept different CosmosMsgs, or the other way around Note might have a cosmosMsg that is not supported on the Voice chain.

5. Will it work with a module? (ica module)? can it even work? specifically talking about chains without wasm.

Probably not the best place to put all those questions, probably I also jump a head by a lot, but I feel like those are important questions that we need to answer to know the course of action we want to take.

[Art3miX]

Thinking more about this, I think its best to stick with instantiate1 in this case on proxy, because this happens over IBC, so you by default waiting for a callback, I don't see any great value in doing that over instantiate2.

If im not mistaken, what you do on L73, you basically return the data field to the callback and ignore the Events, I think this is a mistake, I think it should return the whole data as we got it: collector[msg.id as usize] = res.data.
This will allow the callback to use parse_instantiate_response_data or parse_reply_execute_data based on the called msg, as if they got the reply directly.

[0xekez]

I think it should return the whole data as we got it: collector[msg.id as usize] = res.data

awesome 🙏 this sounds great.

[0xekez]

Thanks so much for these detailed thoughts.

if we take instantiate2 for example, the note contract doesn't need any special treatment, because we can just create custom structs and send it over to the other chain.
But for the voice to execute instatiate2 the chain needs to support it, because if it doesn't, it will error out.

This is a good observation. How about making "instantiate2" a feature in the handshake? This is more-or-less what it was designed for.

I think best course of action with this is to create a UpdateAccount logic where it automatically checks if migration is needed to the account (proxy, based on the voice version) and do the migration if it does needed.
This will ensure only active accounts are doing migrations.

Amazing idea. Could you make a ticket for this? I think the only action now is to make the voice the admin of its proxies. We can follow up with that migration logic in a future version, and migrate to it.

Account timeouts?

I agree with your conclusion here that the solution seems non-obvious, but it is worth documenting and considering. Mind making an issue for this as well?

a. Update handshake to include it as supported

Maybe we just close the channel on migration and then make a new one?

Will it work with a module? (ica module)? can it even work? specifically talking about chains without wasm.

Yes, I am 99% sure can build a wrapper that converts JSON CosmosMsg to proto3 and wraps ICA. This was a big design consideration in the handshake design.

I also jump a head by a lot, but I feel like those are important questions that we need to answer to know the course of action we want to take.

This is good! Thinking far ahead is how we make great software.

[0xekez]

the key to making this instantiate2 optional extension work is that we are clear in the spec that the version JSON may contain additional fields in the TRY and CONFIRM steps. this way a note module can observe the presence of "instantiate2" in the voice version, and use those fields for address generation. if the note would like to require instantiate2 support, it can add the extension to its version.

another way of saying this is that notes express optional preferences by observing the contents of the voice extensions.

[Art3miX]

Will open the issues.

This is a good observation. How about making "instantiate2" a feature in the handshake? This is more-or-less what it was designed for.

This should work for the start, but thinking of the future, I think handshake features should be a set of features instead of single features.
seeing the upgrade rates of chains for the wasm SDK/ ibc or just in general, seems like it will be possible that this list will be long in the future, why I was thinking about following the SDK versioning, to avoid looking at individual features and look more into a standard of support.
But right now none of the options looks the best one, really need to lay out how this will work, seems to be the most important thing in the handshake.

Maybe we just close the channel on migration and then make a new one?

If that doesn't change any addresses (wont effect UI/UX) then sure, probably the optimal way of doing it.

Yes, I am 99% sure can build a wrapper that converts JSON CosmosMsg to proto3 and wraps ICA. This was a big design consideration in the handshake design.

Ok thats good, I think any solution that doesn't work with a module will be limited and unusable in the future. (same goes the opposite way)

[0xekez]

@Art3miX, something else we need to consider for this is how to describe the rules of returning data from remote chains in the Polytone spec. My guess is that the best way would be to say "for JSON-CosmosMsg, data has _ format. Then, we could leave it open for a SDK implementation to encode differently. This is important to have well thought out as we put this data directly into our Callbacks.

[0xekez]

polytone/tests/simtests/functionality_test.go

Lines 443 to 445 in 8be00af

	// Tests that the data returned in a callback contains the address of
	// instantiated contracts and can be accessed by
	// parse_reply_instantiate_data.