Cosmos / CosmWasm agents #2865
Cosmos / CosmWasm agents #2865
Conversation
nambrot
force-pushed
the
trevor/new-featv3-cosmos-oct-28
branch
from
336da71 to
b7c4e2f
Compare
|
|
||
| impl Debug for CosmosMailbox { | ||
| fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { | ||
| // Debug::fmt(&(self as &dyn HyperlaneContract), f) |
There was a problem hiding this comment.
should do this and remove the todo
| /// Returns the module type of the ISM compliant with the corresponding | ||
| /// metadata offchain fetching and onchain formatting standard. | ||
| async fn module_type(&self) -> ChainResult<ModuleType> { | ||
| let query = QueryIsmModuleTypeRequest { |
There was a problem hiding this comment.
should implement the Default trait for QueryIsmModuleTypeRequest (if not already done) and use it here
| impl Mailbox for CosmosMailbox { | ||
| #[instrument(level = "debug", err, ret, skip(self))] | ||
| async fn count(&self, lag: Option<NonZeroU64>) -> ChainResult<u32> { | ||
| let payload = mailbox::NonceRequest { |
There was a problem hiding this comment.
any chance to use Default::default() here?
| .await?; | ||
| let response: mailbox::RecipientIsmResponse = serde_json::from_slice(&data)?; | ||
|
|
||
| // convert Hex to H256 |
There was a problem hiding this comment.
I think this comment is stale
| let response: SimulateResponse = self.provider.wasm_simulate(process_message).await?; | ||
| let result = TxCostEstimate { | ||
| gas_limit: U256::from(response.gas_info.unwrap().gas_used), | ||
| gas_price: U256::from(2500), |
There was a problem hiding this comment.
I'd rather have the 2500 as a constant
| } | ||
|
|
||
| impl CosmosMailboxIndexer { | ||
| /// Create a reference to a mailbox at a specific Ethereum address on some |
There was a problem hiding this comment.
this comment looks stale
### Description Addresses a handful of issues in #2865 ### 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 -->
| pub merkle_hook: T, | ||
| } | ||
|
|
||
| // --------- Requests --------- |
There was a problem hiding this comment.
cleaner to wrap in a mod requests
There was a problem hiding this comment.
actually all of these should be importable from hpl_interface, looking into it
| pub check_point: EmptyStruct, | ||
| } | ||
|
|
||
| // --------- Responses --------- |
There was a problem hiding this comment.
cleaner to wrap in a mod responses
|
|
||
| #[derive(Serialize, Deserialize, Debug)] | ||
| pub struct GetAnnounceStorageLocationsRequest { | ||
| pub get_announce_storage_locations: GetAnnounceStorageLocationsRequestInner, |
There was a problem hiding this comment.
what do we expect the maintenance overhead of these structs to be? wondering if there's a way to generate the expected request/response shape automatically
| use cosmrs::crypto::secp256k1::SigningKey; | ||
| use cosmrs::proto::cosmos::auth::v1beta1::BaseAccount; | ||
| use cosmrs::proto::cosmos::auth::v1beta1::{ | ||
| query_client::QueryClient as QueryAccountClient, QueryAccountRequest, | ||
| }; | ||
| use cosmrs::proto::cosmos::base::abci::v1beta1::TxResponse; | ||
| use cosmrs::proto::cosmos::base::tendermint::v1beta1::{ | ||
| service_client::ServiceClient, GetLatestBlockRequest, | ||
| }; | ||
| use cosmrs::proto::cosmos::tx::v1beta1::service_client::ServiceClient as TxServiceClient; | ||
| use cosmrs::proto::cosmos::tx::v1beta1::{ | ||
| BroadcastMode, BroadcastTxRequest, SimulateRequest, SimulateResponse, | ||
| }; | ||
| use cosmrs::proto::cosmwasm::wasm::v1::{ | ||
| query_client::QueryClient as WasmQueryClient, MsgExecuteContract, | ||
| QuerySmartContractStateRequest, | ||
| }; | ||
| use cosmrs::proto::traits::Message; | ||
|
|
||
| use cosmrs::tx::{self, Fee, MessageExt, SignDoc, SignerInfo}; | ||
| use cosmrs::{Amount, Coin}; |
There was a problem hiding this comment.
added an issue to auto-format these later: #2877
| @@ -367,7 +375,7 @@ impl HyperlaneDomain { | |||
| use HyperlaneDomainProtocol::*; | |||
| let protocol = self.domain_protocol(); | |||
| many_to_one!(match protocol { | |||
| IndexMode::Block: [Ethereum], | |||
| IndexMode::Block: [Ethereum, Cosmos], // TODO: Is cosmos index-mode is correct? | |||
There was a problem hiding this comment.
I believe the answer is yes and we should remove this comment @tkporter
| Ok(sig.recover(hash)?.into()) | ||
| } | ||
|
|
||
| /// Recover the public key of the signer | ||
| #[cfg(feature = "ethers")] | ||
| pub fn recover_pubkey(&self) -> Result<Vec<u8>, crate::HyperlaneProtocolError> { |
There was a problem hiding this comment.
doesn't look like this is used anywhere
|
During testing I noticed that the relayer fetches the entire events list on each block, we should look into this since it's quite inefficient |
| } | ||
|
|
||
| #[derive(Clone)] | ||
| pub struct CosmosConfig { |
There was a problem hiding this comment.
what are codes? I know this code is just for testing but let's add some brief doc comments
| } | ||
|
|
||
| pub fn wasm_query<T: serde::ser::Serialize>( | ||
| // U: serde::de::DeserializeOwned>( |
| // let output: CliWasmQueryResponse<U> = serde_json::from_str(output).unwrap(); | ||
|
|
||
| // output.data |
| // cli.wasm_execute( | ||
| // &network.launch_resp.endpoint, | ||
| // linker, | ||
| // &network.deployments.va, | ||
| // va::ExecuteMsg::Announce { | ||
| // validator: (), | ||
| // storage_location: (), | ||
| // signature: (), | ||
| // }, | ||
| // vec![], | ||
| // ); | ||
| } |
There was a problem hiding this comment.
does e2e not use a multisig ism?
| target_domain: u32, | ||
| ) { | ||
| let validator_addr = validator.addr(hrp); | ||
| let _validator_pubkey = validator.pub_key_to_binary(); |
|
|
||
| impl CodeSource { | ||
| fn install_local(src: String) -> BTreeMap<String, PathBuf> { | ||
| // make contract_name => path map |
There was a problem hiding this comment.
this comment is present repeat three times across the project, and the code it describes looks identical each time. Let's try reusing this function everywhere instead of duplicating
| } | ||
|
|
||
| impl CLISource { | ||
| fn install_remote(dir: Option<PathBuf>, git: String, version: String) -> PathBuf { |
There was a problem hiding this comment.
rename to install_cli?
| message: &HyperlaneMessage, | ||
| metadata: &[u8], | ||
| ) -> ChainResult<Option<U256>> { | ||
| Ok(Some(U256::from(1000))) // TODO |
There was a problem hiding this comment.
and let's double check that aggregation ISM isn't required
| #[instrument(level = "debug", err, ret, skip(self))] | ||
| async fn delivered(&self, id: H256) -> ChainResult<bool> { | ||
| let id = hex::encode(id); | ||
| let payload = mailbox::DeliveredRequest { |
There was a problem hiding this comment.
move to constructor
|
|
||
| let delivered = match self | ||
| .provider | ||
| .wasm_query(GeneralMailboxQuery { mailbox: payload }, None) |
There was a problem hiding this comment.
same should have a more ergonomic interface
rust/hyperlane-core/src/error.rs
Outdated
| @@ -78,6 +84,51 @@ pub enum ChainCommunicationError { | |||
| /// Failed to parse strings or integers | |||
| #[error("Data parsing error {0:?}")] | |||
| StrOrIntParseError(#[from] StrOrIntParseError), | |||
| /// Tendermint RPC Error | |||
There was a problem hiding this comment.
would prefer to not put execution env specific things in here
| Ok(sig.recover(hash)?.into()) | ||
| } | ||
|
|
||
| /// Recover the public key of the signer | ||
| #[cfg(feature = "ethers")] | ||
| pub fn recover_pubkey(&self) -> Result<Vec<u8>, crate::HyperlaneProtocolError> { |
rust/neutron_relayer_config.json
Outdated
| @@ -0,0 +1,30 @@ | |||
| { | |||
rust/neutron_validator_config.json
Outdated
| @@ -0,0 +1,12 @@ | |||
| { | |||
| @@ -0,0 +1,43 @@ | |||
| use k256::ecdsa::{SigningKey, VerifyingKey}; | |||
There was a problem hiding this comment.
maybe unnencessary here
### Description <!-- What's included in this PR? --> ### 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 <!-- What's included in this PR? --> ### 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 * Creates a single channel that's cloned (which can be done [cheaply and is encouraged](https://docs.rs/tonic/latest/tonic/transport/struct.Channel.html)) when creating a new client. * general cleanup & renaming * I believe a chain signer will not be required by e.g. the validator, will confirm this locally ### Drive-by changes n/a ### Related issues n/a ### Backward compatibility yee ### Testing builds, local test
Size of rust build exceeds the storage space in the `ubuntu-latest` runner. `larger-runner` has more storage, but let's see if it also has all the system dependencies required to build
Adds "gas payment count" and "confirmed submitted messages" as termination invariants to the cw run-locally, to get the test to actually fail if messages are not delivered
### Description <!-- What's included in this PR? --> ### 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 <!-- What's included in this PR? --> ### 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