Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a contract address in every call to wasmVM.Instantiate #4928

Closed
Tracked by #4925
crodriguezvega opened this issue Oct 23, 2023 · 4 comments · Fixed by #4939
Closed
Tracked by #4925

Use a contract address in every call to wasmVM.Instantiate #4928

crodriguezvega opened this issue Oct 23, 2023 · 4 comments · Fixed by #4939
Assignees
Labels

Comments

@crodriguezvega
Copy link
Contributor

In order to be able to migrate contract instances we need to instantiate contracts with contract addresses.

Context

In x/wasm contract migration requires the contract to have an address, which is passed in the env parameter. In 08-wasm we currently do not have addresses for the light client contracts. In x/wasm the contract address is generated during contract instantiation, each contract instance gets its own contract address.

In x/wasm there is MsgMigrateContract with a field contract that is the address of the contract that should be migrated. The code_id field references the new contract that migrates to.

Proposal

Investigate if we can use the client identifier as contract address (thanks @srdtrk for this suggestion). The 08-wasm module does not have knowledge of the client identifiers, so we need to investigate if we can retrieve it from the client-prefixed store that is passed to Initialize.

If we cannot use the client identifier as contract address, then we could generate a contract address using the code hash and a similar trick as we do when we generate the interchain account address. In the Initialize function we also have access to the client store, so we could store the contract address under a contracAddress key. We need this for #4925 so that we can retrieve the contract address in MigrateContract.

Regardless of the contract address that we use, it needs to be passed to wasmVM.Instantiate in the env parameter.

@srdtrk
Copy link
Member

srdtrk commented Oct 24, 2023

Some initial thoughts after some research but before experimenting:

  • It seems possible to retrieve the clientID from the prefixed store (will verify experimentally later)
  • It is probably not necessary to pass the clientID or contractAddress to the env. Passing to env only allows the contract to know its own clientID. Regardless, I think we should pass it to env.
  • I don't know why this issue only covers wasmVM.Instantiate. We should be able to pass whatever address we use to Sudo, and Query as well (in the env).

@crodriguezvega
Copy link
Contributor Author

I don't know why this issue only covers wasmVM.Instantiate. We should be able to pass whatever address we use to Sudo, and Query as well (in the env).

You're right! Should be used in the other calls as well. Do you mind to add that as well in your PR?

@srdtrk
Copy link
Member

srdtrk commented Oct 24, 2023

Already in my PR @crodriguezvega. The only issue I faced is having to use reflect to get the value of prefix since it is a private field. (I didn't find any public getters for it)

@crodriguezvega crodriguezvega moved this from Todo to In review in ibc-go Oct 27, 2023
@colin-axner
Copy link
Contributor

closed by #4939, many thanks @srdtrk

@github-project-automation github-project-automation bot moved this from In review to Done in ibc-go Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants