IBC-rate-limiting: modify the contract interface to handle the packet #3234
Conversation
…-limit-integration-and-tests
…limit-integration-and-tests
…te-limit/optional-packet
| @@ -351,7 +351,7 @@ func (appKeepers *AppKeepers) InitNormalKeepers( | |||
|
|
|||
| // The last arguments can contain custom message handlers, and custom query handlers, | |||
| // if we want to allow any custom callbacks | |||
| supportedFeatures := "iterator,staking,stargate,osmosis" | |||
| supportedFeatures := "iterator,staking,stargate,osmosis,cosmwasm_1_1" | |||
There was a problem hiding this comment.
Sanity checking, cosmwasm 1.1 won't break anything existing right
There was a problem hiding this comment.
All CosmWasm 1.0 contracts run on 1.0 and 1.1 hosts. All CosmWasm 1.1 contracts run on 1.0 and 1.1 hosts. Smooth, right?
https://medium.com/cosmwasm/cosmwasm-1-1-14233baf8162
Perfect! (though if 1.1 runs on 1.0 hosts, I don't really understand why its feature flagged?)
There was a problem hiding this comment.
This just makes the feature available so that if someone compiles their contract with 1.1, then the 1.1 features from cosmwasm_std and whatever they feature-gate in their contract will be available.
The version of wasmd we're on should already support cosmwasm_1.1
| #[derive( | ||
| Clone, | ||
| PartialEq, | ||
| Eq, | ||
| ::prost::Message, | ||
| serde::Serialize, | ||
| serde::Deserialize, | ||
| schemars::JsonSchema, | ||
| CosmwasmExt, | ||
| )] | ||
| #[proto_message(type_url = "/cosmos.bank.v1beta1.QuerySupplyOfRequest")] | ||
| #[proto_query( | ||
| path = "/cosmos.bank.v1beta1.Query/SupplyOf", | ||
| response_type = QuerySupplyOfResponse | ||
| )] | ||
| pub struct QuerySupplyOfRequest { | ||
| #[prost(string, tag = "1")] | ||
| pub denom: ::prost::alloc::string::String, | ||
| } |
There was a problem hiding this comment.
Wow, this is very gross.
Mentally flagging that we need to do some things to automate a lot of these derives. (Proto_message) should be able to auto-add
Clone,
PartialEq,
Eq,
::prost::Message,
serde::Serialize,
serde::Deserialize,
schemars::JsonSchema,
cc @pyramation
There was a problem hiding this comment.
We have automation for this on osmosis-rust (which basically auto generates code like this one). That particular query was missing and it didn't generate when I ran the generation code. I passed this along to Boss so it gets added to osmosis-std, but for now I'm inlining it in my contract so we can use it right away
| "any".to_string() // native tokens are rate limited globally | ||
| }; | ||
| fn local_denom(&self) -> String { | ||
| if !self.data.denom.starts_with("transfer/") { |
There was a problem hiding this comment.
oh what, its transfer? I always thought it was ibc/
There was a problem hiding this comment.
ibc/aaa... is the name of the local token created by the transfer module, but the packet contains port/channel/denom (i.e.: transfer/channel-32/osmo). When receiving, the counterparty just sends their local denom, and the receiver hashes the token (after the middleware has handled it). When sending, we send ibc/aaa... but the transfer module "unhashes" it before passing it down to the middleware. So the middleware always sees the denom trace
There was a problem hiding this comment.
ahh, I see. TIL! Can we add a comment somewhere to spec or go source code?
This builds off of #3229.
With this changes the contract's interface now takes the packet so that the information can be extracted in the contract instead of on the chain. This gives us more flexibility later modify how the contract behaves (for instance, how we calculate the channel value) without having to touch the go side.
For compatibility, the calls are temporarily taking a
channel_value_hintand alocal_denomcalculated by the chain. The objective is to remove those, but having them for now allows us to migrate the tests in a safe way to ensure nothing breaks while making these changes.The next steps are: