Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: delegate keypair encoding to cosmrs #2887

Merged
merged 7 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions rust/chains/hyperlane-cosmos/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ version = { workspace = true }
[dependencies]
async-trait = { workspace = true }
cosmrs = { workspace = true, features = ["cosmwasm", "tokio", "grpc", "rpc"] }
derive-new = { workspace = true }
serde = { workspace = true }
thiserror = { workspace = true }
tokio = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion rust/chains/hyperlane-cosmos/src/aggregation_ism.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
address::bech32_decode,
grpc::{WasmGrpcProvider, WasmProvider},
payloads::aggregate_ism::{ModulesAndThresholdRequest, ModulesAndThresholdResponse},
verify::bech32_decode,
ConnectionConf, CosmosProvider, Signer,
};
use async_trait::async_trait;
Expand Down
6 changes: 6 additions & 0 deletions rust/chains/hyperlane-cosmos/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ pub enum HyperlaneCosmosError {
/// gRPC error
#[error("{0}")]
GrpcError(#[from] tonic::Status),
/// Cosmos error
#[error("{0}")]
CosmosError(#[from] cosmrs::Error),
/// Cosmos error report
#[error("{0}")]
CosmosErrorReport(#[from] cosmrs::ErrorReport),
}

impl From<HyperlaneCosmosError> for ChainCommunicationError {
Expand Down
31 changes: 5 additions & 26 deletions rust/chains/hyperlane-cosmos/src/interchain_security_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ use crate::{
grpc::{WasmGrpcProvider, WasmProvider},
payloads::{
general::EmptyStruct,
ism_routes::{
QueryIsmGeneralRequest, QueryIsmModuleTypeRequest, QueryIsmModuleTypeResponse,
},
ism_routes::{QueryIsmGeneralRequest, QueryIsmModuleTypeRequest},
},
types::IsmType,
ConnectionConf, CosmosProvider, Signer,
};

Expand Down Expand Up @@ -57,19 +56,6 @@ impl HyperlaneChain for CosmosInterchainSecurityModule {
}
}

fn ism_type_to_module_type(ism_type: hpl_interface::ism::ISMType) -> ModuleType {
match ism_type {
hpl_interface::ism::ISMType::Unused => ModuleType::Unused,
hpl_interface::ism::ISMType::Routing => ModuleType::Routing,
hpl_interface::ism::ISMType::Aggregation => ModuleType::Aggregation,
hpl_interface::ism::ISMType::LegacyMultisig => ModuleType::MessageIdMultisig,
hpl_interface::ism::ISMType::MerkleRootMultisig => ModuleType::MerkleRootMultisig,
hpl_interface::ism::ISMType::MessageIdMultisig => ModuleType::MessageIdMultisig,
hpl_interface::ism::ISMType::Null => ModuleType::Null,
hpl_interface::ism::ISMType::CcipRead => ModuleType::CcipRead,
}
}

#[async_trait]
impl InterchainSecurityModule for CosmosInterchainSecurityModule {
/// Returns the module type of the ISM compliant with the corresponding
Expand All @@ -84,16 +70,9 @@ impl InterchainSecurityModule for CosmosInterchainSecurityModule {
.wasm_query(QueryIsmGeneralRequest { ism: query }, None)
.await?;

// Handle both the ISMType response and the ModuleTypeResponse response.
let ismtype_response = serde_json::from_slice::<QueryIsmModuleTypeResponse>(&data);
let moduletye_response =
serde_json::from_slice::<hpl_interface::ism::ModuleTypeResponse>(&data);

Ok(match (ismtype_response, moduletye_response) {
(Ok(v), _) => ism_type_to_module_type(v.typ),
(_, Ok(v)) => ism_type_to_module_type(v.typ),
_ => ModuleType::Null,
})
let module_type_response =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😎

serde_json::from_slice::<hpl_interface::ism::ModuleTypeResponse>(&data)?;
Ok(IsmType(module_type_response.typ).into())
}

/// Dry runs the `verify()` ISM call and returns `Some(gas_estimate)` if the call
Expand Down
1 change: 1 addition & 0 deletions rust/chains/hyperlane-cosmos/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod providers;
mod routing_ism;
mod signers;
mod trait_builder;
mod types;
mod utils;
mod validator_announce;

Expand Down
53 changes: 53 additions & 0 deletions rust/chains/hyperlane-cosmos/src/libs/address.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
use std::str::FromStr;

use cosmrs::{crypto::secp256k1::SigningKey, AccountId};
use derive_new::new;
use hyperlane_core::{ChainResult, H256};

/// decode bech32 address to H256
pub fn bech32_decode(addr: String) -> ChainResult<H256> {
let account_id = AccountId::from_str(&addr)?;

// although `from_slice` can panic if the slice is not 32 bytes long,
// we know that we're passing in a value that is 32 bytes long because it was decoded from
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe bech32 is named that because it's the encoding is done in base 32 and not because it's 32 bytes long. I think the pattern in Cosmos is that EOA addresses are 20 bytes and smart contracts are 32 bytes. So we should probably be more defensive here

Looks like account IDs can get up to 255 bytes https://docs.rs/cosmrs/latest/cosmrs/struct.AccountId.html#associatedconstant.MAX_LENGTH

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unit tests broke this, good point

// bech32
Ok(H256::from_slice(&account_id.to_bytes()))
}

/// Wrapper around the cosmrs AccountId type that abstracts keypair conversions and
/// bech32 encoding
#[derive(new)]
pub struct CosmosAddress {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may be a good place to have some quick unit tests if we're uncertain about the encoding / decoding like iiuc you flag in the PR description

account_id: AccountId,
}

impl CosmosAddress {
/// Returns a Bitcoin style address: RIPEMD160(SHA256(pubkey))
/// Source: https://github.com/cosmos/cosmos-sdk/blob/177e7f45959215b0b4e85babb7c8264eaceae052/crypto/keys/secp256k1/secp256k1.go#L154
pub fn from_pubkey(pub_key: &[u8], prefix: &str) -> ChainResult<Self> {
let account_id = AccountId::new(prefix, pub_key)?;
Ok(Self { account_id })
}

/// Creates a wrapper arround a cosmrs AccountId from a private key byte array
pub fn from_privkey(priv_key: &[u8], prefix: &str) -> ChainResult<Self> {
let pubkey = SigningKey::from_slice(priv_key)?.public_key().to_bytes();
Self::from_pubkey(&pubkey, prefix)
}

/// Creates a wrapper arround a cosmrs AccountId from a H256 digest
pub fn from_h256(digest: H256, prefix: &str) -> ChainResult<Self> {
let bytes = digest.as_bytes();
CosmosAddress::from_pubkey(bytes, prefix)
}

/// String representation of a cosmos AccountId
pub fn address(&self) -> String {
self.account_id.to_string()
}
}

/// encode H256 to bech32 address
pub fn pub_to_addr(pub_key: Vec<u8>, prefix: &str) -> ChainResult<String> {
Ok(CosmosAddress::from_pubkey(&pub_key, prefix)?.address())
}
28 changes: 0 additions & 28 deletions rust/chains/hyperlane-cosmos/src/libs/binary.rs

This file was deleted.

5 changes: 1 addition & 4 deletions rust/chains/hyperlane-cosmos/src/libs/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,2 @@
/// This module contains all the verification variables the libraries used by the Hyperlane Cosmos chain.
pub mod verify;

/// This module contains all the Binary variables used by the Hyperlane Cosmos chain.
pub mod binary;
pub mod address;
132 changes: 0 additions & 132 deletions rust/chains/hyperlane-cosmos/src/libs/verify.rs

This file was deleted.

12 changes: 5 additions & 7 deletions rust/chains/hyperlane-cosmos/src/mailbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@ use std::io::Cursor;
use std::num::NonZeroU64;
use std::ops::RangeInclusive;

use crate::address::CosmosAddress;
use crate::grpc::{WasmGrpcProvider, WasmProvider};
use crate::payloads::mailbox::{
GeneralMailboxQuery, ProcessMessageRequest, ProcessMessageRequestInner,
};
use crate::payloads::{general, mailbox};
use crate::rpc::{CosmosWasmIndexer, WasmIndexer};
use crate::CosmosProvider;
use crate::{signers::Signer, utils::get_block_height_for_lag, verify, ConnectionConf};
use crate::{address, signers::Signer, utils::get_block_height_for_lag, ConnectionConf};
use async_trait::async_trait;

use cosmrs::proto::cosmos::base::abci::v1beta1::TxResponse;
use cosmrs::proto::cosmos::tx::v1beta1::SimulateResponse;
use cosmrs::tendermint::abci::EventAttribute;

use crate::binary::h256_to_h512;
use hyperlane_core::{
utils::fmt_bytes, ChainResult, HyperlaneChain, HyperlaneContract, HyperlaneDomain,
HyperlaneMessage, HyperlaneProvider, Indexer, LogMeta, Mailbox, TxCostEstimate, TxOutcome,
Expand Down Expand Up @@ -134,7 +134,7 @@ impl Mailbox for CosmosMailbox {

#[instrument(err, ret, skip(self))]
async fn recipient_ism(&self, recipient: H256) -> ChainResult<H256> {
let address = verify::digest_to_addr(recipient, &self.signer.prefix)?;
let address = CosmosAddress::from_h256(self.address, &self.signer.prefix)?.address();

let payload = mailbox::RecipientIsmRequest {
recipient_ism: mailbox::RecipientIsmRequestInner {
Expand All @@ -149,7 +149,7 @@ impl Mailbox for CosmosMailbox {
let response: mailbox::RecipientIsmResponse = serde_json::from_slice(&data)?;

// convert Hex to H256
let ism = verify::bech32_decode(response.ism)?;
let ism = address::bech32_decode(response.ism)?;
Ok(ism)
}

Expand All @@ -172,9 +172,7 @@ impl Mailbox for CosmosMailbox {
.wasm_send(process_message, tx_gas_limit)
.await?;
Ok(TxOutcome {
transaction_id: h256_to_h512(H256::from_slice(
hex::decode(response.txhash)?.as_slice(),
)),
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),
Expand Down
Loading
Loading