consider contract address length for instantiate2 #97
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
taitruong
commented
May 30, 2024
packages/ics721/src/helpers.rs
Outdated
Comment on lines
154
to
160
| if let Some(len) = CONTRACT_ADDR_LENGTH.may_load(deps.storage)? { | ||
| Ok(deps | ||
| .api | ||
| .addr_humanize(&canonical_cw721_addr[..len].into())?) | ||
| } else { | ||
| Ok(deps.api.addr_humanize(&canonical_cw721_addr)?) | ||
| } |
There was a problem hiding this comment.
Here is the main logic for slicing addr
There was a problem hiding this comment.
@humanalgorithm look here, for Injective CONTRACT_ADDR_LENGTH is set with 20. All other chains have no entry in CONTRACT_ADDR_LENGTH storage.
Value is then used for slicing first 20 bytes: canonical_cw721_addr[..len]
packages/ics721/src/msg.rs
Outdated
| @@ -31,6 +31,8 @@ pub struct InstantiateMsg { | |||
| pub pauser: Option<String>, | |||
| /// The admin address for instantiating new cw721 contracts. In case of None, contract is immutable. | |||
| pub cw721_admin: Option<String>, | |||
| /// The optional contract address length being used for instantiate2. In case of None, default length is 32 (standard in cosmwasm). | |||
| pub contract_addr_length: Option<usize>, | |||
There was a problem hiding this comment.
usize has different sizes in dev and production. Better use u32 here
There was a problem hiding this comment.
fixed and changed to u32
humanalgorithm
approved these changes
May 31, 2024
jhernandezb
approved these changes
May 31, 2024
shanev
approved these changes
May 31, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds new
CONTRACT_ADDR_LENGTH: Item<usize> = Item::new("n")storage. In most case it can be left empty. For Injective it contains20as value.By default cosmwasm considers addresses with 32 bytes as a constant. Injective on the other hand is of length 20 bytes.
See details here: CosmWasm/cosmwasm#2155
So
instantiate2fails, when transferring from a chain to Injective. Solution is simple - as suggested by Simon: slicing and taking first 20 bytes.This has been tested on testnet, Stargaze -> Injective, and works like a charm.