-
Notifications
You must be signed in to change notification settings - Fork 344
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
Add WasmQuery::CodeInfo to get checksum for code ID #1561
Conversation
3501878
to
c6505ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and useful!
Please see my comment about the InstantiateConfig
|
||
/// The essential data from wasmd's [CodeInfo]/[CodeInfoResponse]. | ||
/// | ||
/// `code_hash`/`data_hash` was renamed to `checksum` to follow the CosmWasm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 good to have the naming correct here.
pub creator: String, | ||
/// The hash of the Wasm blob | ||
pub checksum: Binary, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to return the InstantiateConfig
as well? It feels incomplete without
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hesitant to add it since it is a complex deeply nested type that is not trivial to add to all the layers in between. But I can have a second look today.
I agree it feels incomplete. But on the other hand the smaller the interface, the more we can break on the wasmd side without breaking contracts. Do you see a clear use case for this information in the contract?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that
- there are still things moving around instantiation permissions (Add AccesType OnlyCodeIDs wasmd#1042, Remove ACCESS_TYPE_ONLY_ADDRESS? wasmd#1142),
- we currently do not have experience with bringing enums from protobuf via JSON to Rust,
- the type is complex with address fields only being valid in case of certain enum values
I would like to avoid blockin Instantiate2 usage on that. The field can be added later on.
@@ -26,6 +26,9 @@ pub enum WasmQuery { | |||
}, | |||
/// Returns a [`ContractInfoResponse`] with metadata on the contract from the runtime | |||
ContractInfo { contract_addr: String }, | |||
/// Returns a [`CodeInfoResponse`] with metadata of the code | |||
#[cfg(feature = "cosmwasm_1_2")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
08a2bf7
to
224b80c
Compare
The motivation for this addition is the following:
WasmMsg::Instantiate2
requires a code ID. But when a contract wants to pre-compute the address, it also needs the checksum. It would be very inconvenient if a contract developer had to provide both as part of the instruction. With the query you can just provide a code ID and query the checksum as part of the contract.