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

Ethermint support #1295

Merged
merged 8 commits into from
Sep 10, 2021
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
4 changes: 4 additions & 0 deletions .changelog/unreleased/features/1267-ethermint-support.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Added Ethermint support ([#1267] [#1071])

[#1267]: https://github.com/informalsystems/ibc-rs/issues/1267
[#1071]: https://github.com/informalsystems/ibc-rs/issues/1071
16 changes: 16 additions & 0 deletions Cargo.lock

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

15 changes: 15 additions & 0 deletions config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,20 @@ trusting_period = '14days'
# Warning: This is an advanced feature! Modify with caution.
trust_threshold = { numerator = '1', denominator = '3' }

# Specify the address type which determines:
adizere marked this conversation as resolved.
Show resolved Hide resolved
# 1) address derivation;
# 2) how to retrieve and decode accounts and pubkeys;
# 3) the message signing method.
# The current configuration options are for Cosmos SDK and Ethermint.
#
# Example configuration for Ethermint:
#
# address_type = { derivation = 'ethermint', proto_type = { pk_type = '/injective.crypto.v1beta1.ethsecp256k1.PubKey' } }
#
# Default: { derivation = 'cosmos' }, i.e. address derivation as in Cosmos SDK
# Warning: This is an advanced feature! Modify with caution.
address_type = { derivation = 'cosmos' }

# This section specifies the filters for policy based relaying.
# Default: no policy/ filters
# The section is ignored if the global 'filter' option is set to 'false'.
Expand Down Expand Up @@ -168,3 +182,4 @@ max_tx_size = 2097152
clock_drift = '5s'
trusting_period = '14days'
trust_threshold = { numerator = '1', denominator = '3' }
address_type = { derivation = 'cosmos' }
8 changes: 8 additions & 0 deletions proto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ pub mod cosmos {
pub mod auth {
pub mod v1beta1 {
include!("prost/cosmos.auth.v1beta1.rs");
/// EthAccount defines an Ethermint account.
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct EthAccount {
#[prost(message, optional, tag = "1")]
pub base_account: ::core::option::Option<BaseAccount>,
#[prost(bytes = "vec", tag = "2")]
pub code_hash: ::prost::alloc::vec::Vec<u8>,
}
}
}
pub mod staking {
Expand Down
2 changes: 1 addition & 1 deletion relayer-cli/src/commands/keys/restore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ pub fn restore_key(
config: &ChainConfig,
) -> Result<KeyEntry, Box<dyn std::error::Error>> {
let mut keyring = KeyRing::new(Store::Test, &config.account_prefix, &config.id)?;
let key_entry = keyring.key_from_mnemonic(mnemonic, hdpath)?;
let key_entry = keyring.key_from_mnemonic(mnemonic, hdpath, &config.address_type)?;

keyring.add_key(key_name, key_entry.clone())?;
Ok(key_entry)
Expand Down
1 change: 1 addition & 0 deletions relayer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ bitcoin = { version = "=0.27", features = ["use-serde"] }
tiny-bip39 = "0.8.0"
hdpath = { version = "0.6.0", features = ["with-bitcoin"] }
sha2 = "0.9.3"
tiny-keccak = { version = "2.0.2", features = ["keccak"], default-features = false }
ripemd160 = "0.9.1"
bech32 = "0.8.1"
itertools = "0.10.1"
Expand Down
69 changes: 42 additions & 27 deletions relayer/src/chain/cosmos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ use ibc::ics24_host::{ClientUpgradePath, Path, IBC_QUERY_PATH, SDK_UPGRADE_QUERY
use ibc::query::{QueryTxHash, QueryTxRequest};
use ibc::signer::Signer;
use ibc::Height as ICSHeight;
use ibc_proto::cosmos::auth::v1beta1::{BaseAccount, QueryAccountRequest};
use ibc_proto::cosmos::auth::v1beta1::{BaseAccount, EthAccount, QueryAccountRequest};
use ibc_proto::cosmos::base::tendermint::v1beta1::service_client::ServiceClient;
use ibc_proto::cosmos::base::tendermint::v1beta1::GetNodeInfoRequest;
use ibc_proto::cosmos::base::v1beta1::Coin;
Expand All @@ -71,7 +71,7 @@ use ibc_proto::ibc::core::connection::v1::{
QueryClientConnectionsRequest, QueryConnectionsRequest,
};

use crate::config::{ChainConfig, GasPrice};
use crate::config::{AddressType, ChainConfig, GasPrice};
use crate::error::Error;
use crate::event::monitor::{EventMonitor, EventReceiver};
use crate::keyring::{KeyEntry, KeyRing, Store};
Expand Down Expand Up @@ -523,15 +523,15 @@ impl CosmosSdkChain {
fn account(&mut self) -> Result<&mut BaseAccount, Error> {
if self.account == None {
let account = self.block_on(query_account(self, self.key()?.account))?;

debug!(
sequence = %account.sequence,
number = %account.account_number,
"[{}] send_tx: retrieved account",
self.id()
);

self.account = Some(account);
if let Some(acc) = account.as_ref() {
debug!(
sequence = %acc.sequence,
number = %acc.account_number,
"[{}] send_tx: retrieved account",
self.id()
);
}
self.account = account;
tomtau marked this conversation as resolved.
Show resolved Hide resolved
}

Ok(self
Expand All @@ -555,9 +555,13 @@ impl CosmosSdkChain {

fn signer(&self, sequence: u64) -> Result<SignerInfo, Error> {
let (_key, pk_buf) = self.key_and_bytes()?;
let pk_type = match &self.config.address_type {
AddressType::Cosmos => "/cosmos.crypto.secp256k1.PubKey".to_string(),
AddressType::Ethermint { pk_type } => pk_type.clone(),
};
// Create a MsgSend proto Any message
let pk_any = Any {
type_url: "/cosmos.crypto.secp256k1.PubKey".to_string(),
type_url: pk_type,
value: pk_buf,
};

Expand Down Expand Up @@ -610,7 +614,11 @@ impl CosmosSdkChain {
// Sign doc
let signed = self
.keybase
.sign_msg(&self.config.key_name, signdoc_buf)
.sign_msg(
&self.config.key_name,
signdoc_buf,
&self.config.address_type,
)
.map_err(Error::key_base)?;

Ok(signed)
Expand Down Expand Up @@ -1888,7 +1896,10 @@ async fn broadcast_tx_sync(chain: &CosmosSdkChain, data: Vec<u8>) -> Result<Resp
}

/// Uses the GRPC client to retrieve the account sequence
async fn query_account(chain: &CosmosSdkChain, address: String) -> Result<BaseAccount, Error> {
async fn query_account(
chain: &CosmosSdkChain,
address: String,
) -> Result<Option<BaseAccount>, Error> {
let mut client = ibc_proto::cosmos::auth::v1beta1::query_client::QueryClient::connect(
chain.grpc_addr.clone(),
)
Expand All @@ -1898,19 +1909,23 @@ async fn query_account(chain: &CosmosSdkChain, address: String) -> Result<BaseAc
let request = tonic::Request::new(QueryAccountRequest { address });

let response = client.account(request).await;

let base_account = BaseAccount::decode(
response
.map_err(Error::grpc_status)?
.into_inner()
.account
.unwrap()
.value
.as_slice(),
)
.map_err(|e| Error::protobuf_decode("BaseAccount".to_string(), e))?;

Ok(base_account)
let resp_account = response
.map_err(Error::grpc_status)?
.into_inner()
.account
.unwrap();
if resp_account.type_url == "/cosmos.auth.v1beta1.BaseAccount" {
Ok(Some(
BaseAccount::decode(resp_account.value.as_slice())
.map_err(|e| Error::protobuf_decode("BaseAccount".to_string(), e))?,
))
} else if resp_account.type_url.ends_with(".EthAccount") {
Ok(EthAccount::decode(resp_account.value.as_slice())
.map_err(|e| Error::protobuf_decode("EthAccount".to_string(), e))?
.base_account)
} else {
Err(Error::unknown_account_type(resp_account.type_url))
}
}

fn encode_to_bech32(address: &str, account_prefix: &str) -> Result<String, Error> {
Expand Down
3 changes: 2 additions & 1 deletion relayer/src/chain/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ pub mod test_utils {

use ibc::ics24_host::identifier::ChainId;

use crate::config::{ChainConfig, GasPrice, PacketFilter};
use crate::config::{AddressType, ChainConfig, GasPrice, PacketFilter};

/// Returns a very minimal chain configuration, to be used in initializing `MockChain`s.
pub fn get_basic_chain_config(id: &str) -> ChainConfig {
Expand All @@ -413,6 +413,7 @@ pub mod test_utils {
trusting_period: Duration::from_secs(14 * 24 * 60 * 60), // 14 days
trust_threshold: Default::default(),
packet_filter: PacketFilter::default(),
address_type: AddressType::default(),
}
}
}
29 changes: 29 additions & 0 deletions relayer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,33 @@ impl Default for RestConfig {
}
}

#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
#[serde(
rename_all = "lowercase",
tag = "derivation",
content = "proto_type",
deny_unknown_fields
)]
pub enum AddressType {
Cosmos,
Ethermint { pk_type: String },
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if it's a good idea to restrict the possible values that pk_type can take, either by making this an enum, or checking the String against a predefined list of accepted values.

Beside '/injective.crypto.v1beta1.ethsecp256k1.PubKey' and "/ethermint.crypto.v1alpha1.ethsecp256k1.PubKey" are there numerous other values that pk_type can take?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are potentially many more : /ethermint.crypto.v1beta1.ethsecp256k1.PubKey, /ethermint.crypto.v1.ethsecp256k1.PubKey, /cosmos.crypto.ethsecp256k1.PubKey...

Right now, in order to be useful (given Ethermint has been copied over and modified), it makes sense to keep it a bit flexible -- otherwise, every chain would need to open a PR to modify the enum.

Later, once the ethsecp256k1 support lands in Cosmos SDK, it'll make sense to deprecate this with that Cosmos SDK release and recommend chains to migrate to /cosmos.crypto.ethsecp256k1.PubKey (or whatever the canonical proto ends up being)... and after that, remove this extra config.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds like a plan, thanks for the detailed rationale! I agree with you.

}

impl Default for AddressType {
fn default() -> Self {
AddressType::Cosmos
}
}

impl fmt::Display for AddressType {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
AddressType::Cosmos => write!(f, "cosmos"),
AddressType::Ethermint { .. } => write!(f, "ethermint"),
}
}
}

#[derive(Clone, Debug, Deserialize, Serialize)]
#[serde(deny_unknown_fields)]
pub struct ChainConfig {
Expand Down Expand Up @@ -291,6 +318,8 @@ pub struct ChainConfig {
pub gas_price: GasPrice,
#[serde(default)]
pub packet_filter: PacketFilter,
#[serde(default)]
pub address_type: AddressType,
}

/// Attempt to load and parse the TOML config file as a `Config`.
Expand Down
10 changes: 10 additions & 0 deletions relayer/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,16 @@ define_error! {
format!("Hermes health check failed while verifying the application compatibility for chain {0}:{1}; caused by: {2}",
e.chain_id, e.address, e.cause)
},

UnknownAccountType
{
type_url: String
}
|e| {
format!("Failed to deserialize account of an unknown protobuf type: {0}",
e.type_url)
},

}
}

Expand Down
Loading