Optimize validators storage #61
Conversation
maurolacy
requested review from
ethanfrey and
hashedone
and removed request for
ethanfrey
| let admin_res = ADMIN.get(deps.as_ref()); | ||
| if let Err(err) = admin_res { | ||
| let not_found = matches!(err, StdError::NotFound { .. }); | ||
| if !not_found { | ||
| return Err(ContractError::AdminError(AdminError::NotAdmin {})); | ||
| } | ||
| } else { | ||
| return Err(ContractError::AdminError(AdminError::NotAdmin {})); | ||
| } |
There was a problem hiding this comment.
There's probably a simpler / nicer way to do this.
There was a problem hiding this comment.
we should add such method to admin.
There was a problem hiding this comment.
wait... if this is only for tests, we do not want this in production build at all.
how about feature-gating it with #[cfg(features = "integration")] and you make such a build for the wasm used in integration test? Then no worries about it as this code path isn't in production at all
There was a problem hiding this comment.
Good idea. Will do this tomorrow.
There was a problem hiding this comment.
Forgot to remove this in the first iteration.
Then, tried to do it in a follow-up, but hit dependency / feature problems with compiling the wasm using the integration feature.
I'll post my working copy and we can see if there's a way around those problems (other than keeping / removing the entry point entirely).
Update: See #77.
Requires "tgrade" cosmwasm-vm feature
maurolacy
force-pushed
the
42-optimize-validators-storage
branch
from
48f42d5 to
de18d26
Compare
| let admin_res = ADMIN.get(deps.as_ref()); | ||
| if let Err(err) = admin_res { | ||
| let not_found = matches!(err, StdError::NotFound { .. }); | ||
| if !not_found { | ||
| return Err(ContractError::AdminError(AdminError::NotAdmin {})); | ||
| } | ||
| } else { | ||
| return Err(ContractError::AdminError(AdminError::NotAdmin {})); | ||
| } |
There was a problem hiding this comment.
we should add such method to admin.
| let admin_res = ADMIN.get(deps.as_ref()); | ||
| if let Err(err) = admin_res { | ||
| let not_found = matches!(err, StdError::NotFound { .. }); | ||
| if !not_found { | ||
| return Err(ContractError::AdminError(AdminError::NotAdmin {})); | ||
| } | ||
| } else { | ||
| return Err(ContractError::AdminError(AdminError::NotAdmin {})); | ||
| } |
There was a problem hiding this comment.
wait... if this is only for tests, we do not want this in production build at all.
how about feature-gating it with #[cfg(features = "integration")] and you make such a build for the wasm used in integration test? Then no worries about it as this code path isn't in production at all
maurolacy
force-pushed
the
42-optimize-validators-storage
branch
from
189a67b to
db375ac
Compare
| use tgrade_valset::msg::ExecuteMsg; | ||
| use tgrade_valset::state::ValidatorInfo; | ||
|
|
||
| // Copied from test_helpers | ||
| // returns a list of addresses that are set in the tg4-stake contract | ||
| fn addrs(count: u32) -> Vec<String> { | ||
| (1..=count).map(|x| format!("operator-{:03}", x)).collect() | ||
| (1..=count) | ||
| .map(|x| bech32::encode("tgrade", format!("operator-{:03}", x).to_base32(), Variant::Bech32).unwrap()) |
There was a problem hiding this comment.
This is getting closer, but not realistic.
The two cases I refer to are
(1) 20 unencoded bytes before the bech32
(2) 32 unencoded bytes before the bech32
I would replace format!("operator-{:03}", x).to_base32() with
format!("20-byte-operator-{:03}", x) which is 20 bytes
I guess we can skip the 32 byte variant for now as it is unlikely anyone uses contracts, but it would be interesting to test as a true upper limit
Closes #42.
After a lot of wrestling, added a couple integration tests to see
Itemfail.I was determined to do this as part of
tgrade-valsetintegration tests, as I think this might be useful in the future. That required adding the "tgrade" feature to cosmwasm-vm. Will publish that in a cosmwasm PR (CosmWasm/cosmwasm#1208) and we decide what to do with it there.Creating as Draft, as this is just the failure case at the moment. Will work in the proposed fix next.Of course, tests will not pass until a new cosmwasm-vm supporting the "tgrade" feature is released.