Dan/more cw agent remediations #2912
Conversation
daniel-savu
changed the base branch from
main
to
trevor/new-featv3-cosmos-oct-28
daniel-savu
force-pushed
the
dan/more-cw-agent-remediations
branch
from
3148a0d to
a2748c7
Compare
daniel-savu
force-pushed
the
dan/more-cw-agent-remediations
branch
from
a2748c7 to
396ba4c
Compare
rust/Cargo.toml
Outdated
There was a problem hiding this comment.
I used taplo to reorder dependencies alphabetically, looks like it reordered all the keys alphabetically too
| err: &mut ConfigParsingError, | ||
| default_rpc_consensus_type: &str, | ||
| ) -> Option<ChainConnectionConf> { | ||
| if rpcs.len() <= 1 { |
There was a problem hiding this comment.
do we need to handle the case of it being len 0 where we'd I guess return None?
There was a problem hiding this comment.
we are returning None on len 0 here, because next() will return None and map doesn't apply the closure to None
There was a problem hiding this comment.
however I agree this implementation could be cleaner, there's an unwrap in the else branch which relies on the assumption that we're checking the length. Trying to make this nicer
| transaction_id: H256::from_slice(hex::decode(response.txhash)?.as_slice()).into(), | ||
| executed: response.code == 0, | ||
| gas_used: U256::from(response.gas_used), | ||
| gas_price: U256::from(response.gas_wanted), |
There was a problem hiding this comment.
I know this wasn't your change but the gas wanted is the gas limit for tx and not the gas price
I assume this was originally set like this because it wasn't available in TxResponse - imo it's fine to just set it to 1 or something for now (or to get it from the ceil of here https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/trevor/new-featv3-cosmos-oct-28/rust/chains/hyperlane-cosmos/src/providers/grpc.rs#L36, which is how we set the gas price atm - unfortunately there doesn't seem to be a reasonable way of getting it via RPC so atm it's just hardcoded :/), but to add a comment about it
There was a problem hiding this comment.
I see :/ DEFAULT_GAS_PRICE isn't available here because it lives in hyperlane-cosmos, and the TryFrom implementation has to live in this crate due to the orphan rule. I'll just set it to U256::one()
…e indexing fix (#2957) ### Description Image from the latest PR to trevor/new-featv3-cosmos-oct-28, which is #2912 This includes a fix to merkle tree indexing so it indexes it in the same way as messages ### Drive-by changes <!-- Are there any minor or drive-by changes also included? --> ### Related issues <!-- - Fixes #[issue number here] --> ### Backward compatibility <!-- Are these changes backward compatible? Are there any infrastructure implications, e.g. changes that would prohibit deploying older commits using this infra tooling? Yes/No --> ### Testing <!-- What kind of testing have these changes undergone? None/Manual/Unit Tests -->
Description
Drive-by changes
Related issues
Backward compatibility
Testing