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

Add chain parameters for the Ethereum bridge #575

Merged
merged 12 commits into from
Oct 24, 2022
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Configuration settings to do with the Ethereum bridge.
//! Runtime configuration for a validator node.
#[allow(unused_imports)]
use namada::types::ethereum_events::EthereumEvent;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -28,7 +28,8 @@ pub enum Mode {

#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct Config {
/// The mode in which to run the Ethereum bridge
/// The mode in which to run the Ethereum node and oracle setup of this
/// validator.
pub mode: Mode,
/// The Ethereum JSON-RPC endpoint that the Ethereum event oracle will use
/// to listen for events from the Ethereum bridge smart contracts
Expand Down
2 changes: 2 additions & 0 deletions apps/src/lib/config/ethereum_bridge/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pub mod ledger;
pub mod params;
115 changes: 115 additions & 0 deletions apps/src/lib/config/ethereum_bridge/params.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
//! Blockchain-level parameters for the configuration of the Ethereum bridge.
use std::num::NonZeroU64;

use serde::{Deserialize, Serialize};

/// Represents a configuration value for an Ethereum address.
///
/// For instance:
/// `0x6B175474E89094C44Da98b954EedeAC495271d0F`
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[repr(transparent)]
pub struct Address(String);

/// Represents a configuration value for the minimum number of
/// confirmations an Ethereum event must reach before it can be acted on.
#[derive(Clone, Copy, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[repr(transparent)]
pub struct MinimumConfirmations(NonZeroU64);

impl Default for MinimumConfirmations {
fn default() -> Self {
// SAFETY: The only way the API contract of `NonZeroU64` can be violated
// is if we construct values of this type using 0 as argument.
Self(unsafe { NonZeroU64::new_unchecked(100) })
james-chf marked this conversation as resolved.
Show resolved Hide resolved
}
}

/// Represents chain parameters for the Ethereum bridge.
#[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)]
pub struct GenesisConfig {
/// Minimum number of confirmations needed to trust an Ethereum branch.
/// This must be at least one.
pub min_confirmations: MinimumConfirmations,
/// The addresses of the Ethereum contracts that need to be directly known
/// by validators.
pub contracts: Contracts,
}

/// Represents all the Ethereum contracts that need to be directly know about by
/// validators.
#[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)]
pub struct Contracts {
/// The Ethereum address of the ERC20 contract that represents this chain's
/// native token.
pub native_erc20: Address,
/// The Ethereum address of the bridge contract.
pub bridge: UpgradeableContract,
/// The Ethereum address of the governance contract.
pub governance: UpgradeableContract,
}

/// Represents a configuration value for the version of a contract that can be
/// upgraded. Starts from 1.
#[derive(Clone, Copy, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[repr(transparent)]
pub struct ContractVersion(NonZeroU64);

impl Default for ContractVersion {
fn default() -> Self {
// SAFETY: The only way the API contract of `NonZeroU64` can be
// violated is if we construct values of this type using 0 as
// argument.
Self(unsafe { NonZeroU64::new_unchecked(1) })
sug0 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Amazes me that this type does not implement Default.

}
}

/// Represents an Ethereum contract that may be upgraded.
#[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)]
pub struct UpgradeableContract {
/// The Ethereum address of the contract.
pub address: Address,
/// The version of the contract. Starts from 1.
pub version: ContractVersion,
}

#[cfg(test)]
mod tests {
use eyre::Result;

use super::*;

/// Ensure we can serialize and deserialize a [`Config`] struct to and from
/// TOML. This can fail if complex fields are ordered before simple fields
/// in any of the config structs.
#[test]
fn test_round_trip_toml_serde() -> Result<()> {
let config = GenesisConfig {
min_confirmations: MinimumConfirmations::default(),
contracts: Contracts {
native_erc20: Address(
"0x1721b337BBdd2b11f9Eef736d9192a8E9Cba5872".to_string(),
),
bridge: UpgradeableContract {
address: Address(
"0x237d915037A1ba79365E84e2b8574301B6D25Ea0"
.to_string(),
),
version: ContractVersion::default(),
},
governance: UpgradeableContract {
address: Address(
"0x308728EEa73538d0edEfd95EF148Eb678F71c71D"
.to_string(),
),
version: ContractVersion::default(),
},
},
};
let serialized = toml::to_string(&config)?;
let deserialized: GenesisConfig = toml::from_str(&serialized)?;

assert_eq!(config, deserialized);
Ok(())
}
}
4 changes: 4 additions & 0 deletions apps/src/lib/config/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub mod genesis_config {
EstablishedAccount, Genesis, ImplicitAccount, TokenAccount, Validator,
};
use crate::cli;
use crate::config::ethereum_bridge;

#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct HexString(pub String);
Expand Down Expand Up @@ -121,6 +122,9 @@ pub mod genesis_config {
pub gov_params: GovernanceParamsConfig,
// Treasury parameters
pub treasury_params: TreasuryParamasConfig,
// Ethereum bridge config
pub ethereum_bridge_params:
Option<ethereum_bridge::params::GenesisConfig>,
// Wasm definitions
pub wasm: HashMap<String, WasmConfig>,
}
Expand Down
6 changes: 3 additions & 3 deletions apps/src/lib/config/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Node and client configuration

pub mod ethereum;
pub mod ethereum_bridge;
pub mod genesis;
pub mod global;
pub mod utils;
Expand Down Expand Up @@ -83,7 +83,7 @@ pub struct Ledger {
pub chain_id: ChainId,
pub shell: Shell,
pub tendermint: Tendermint,
pub ethereum: ethereum::Config,
pub ethereum_bridge: ethereum_bridge::ledger::Config,
Copy link
Member

Choose a reason for hiding this comment

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

Oh wait, you do have this as a config. We can't configure minimum confirmations or addresses in a normal config. They have to be set at genesis and changed only with governance votes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah okay that makes sense, since you can run into consensus failures if two validators have different ideas of what the min number of blocks should be for an event to be considered as valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the Tendermint v0.37 design, we shouldn't get consensus failures if validators have different ideas of min confirmations. For the addresses of the contracts, those should change only via hard fork as far as I understand it, if we do a contract upgrade, though technically it would be possible for a a governance proposal to change them (right now as the code currently is).

}

#[derive(Clone, Debug, Serialize, Deserialize)]
Expand Down Expand Up @@ -197,7 +197,7 @@ impl Ledger {
),
instrumentation_namespace: "anoman_tm".to_string(),
},
ethereum: ethereum::Config::default(),
ethereum_bridge: ethereum_bridge::ledger::Config::default(),
}
}

Expand Down
20 changes: 12 additions & 8 deletions apps/src/lib/node/ledger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use self::abortable::AbortableSpawner;
use self::ethereum_node::eth_fullnode;
use self::shims::abcipp_shim::AbciService;
use crate::config::utils::num_of_threads;
use crate::config::{ethereum, TendermintMode};
use crate::config::{ethereum_bridge, TendermintMode};
use crate::facade::tendermint_proto::abci::CheckTxType;
use crate::facade::tower_abci::{response, split, Server};
use crate::node::ledger::broadcaster::Broadcaster;
Expand Down Expand Up @@ -622,25 +622,26 @@ async fn maybe_start_ethereum_oracle(
return (None, spawn_dummy_task(()));
}

let ethereum_url = config.ethereum.oracle_rpc_endpoint.clone();
let ethereum_url = config.ethereum_bridge.oracle_rpc_endpoint.clone();

// Start the oracle for listening to Ethereum events
let (eth_sender, eth_receiver) = mpsc::channel(ORACLE_CHANNEL_BUFFER_SIZE);
let oracle = match config.ethereum.mode {
ethereum::Mode::Managed | ethereum::Mode::Remote => {
let oracle = match config.ethereum_bridge.mode {
ethereum_bridge::ledger::Mode::Managed
| ethereum_bridge::ledger::Mode::Remote => {
ethereum_node::oracle::run_oracle(
ethereum_url,
eth_sender,
abort_sender,
)
}
ethereum::Mode::EventsEndpoint => {
ethereum_bridge::ledger::Mode::EventsEndpoint => {
ethereum_node::test_tools::events_endpoint::serve(
eth_sender,
abort_sender,
)
}
ethereum::Mode::Off => spawn_dummy_task(()),
ethereum_bridge::ledger::Mode::Off => spawn_dummy_task(()),
};

(Some(eth_receiver), oracle)
Expand All @@ -659,14 +660,17 @@ async fn maybe_start_geth(
tokio::sync::oneshot::Sender<()>,
) {
if !matches!(config.tendermint.tendermint_mode, TendermintMode::Validator)
|| !matches!(config.ethereum.mode, ethereum::Mode::Managed)
|| !matches!(
config.ethereum_bridge.mode,
ethereum_bridge::ledger::Mode::Managed
)
{
let eth_node = spawn_dummy_task(Ok(()));
let (abort_sender, _) = tokio::sync::oneshot::channel::<()>();
return (eth_node, abort_sender);
}

let ethereum_url = config.ethereum.oracle_rpc_endpoint.clone();
let ethereum_url = config.ethereum_bridge.oracle_rpc_endpoint.clone();

// Boot up geth and wait for it to finish syncing
let (eth_node, abort_sender) =
Expand Down
4 changes: 2 additions & 2 deletions tests/src/e2e/ledger_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::time::{Duration, Instant};
use borsh::BorshSerialize;
use color_eyre::eyre::Result;
use namada::types::token;
use namada_apps::config::ethereum;
use namada_apps::config::ethereum_bridge;
use namada_apps::config::genesis::genesis_config::{
GenesisConfig, ParametersConfig, PosParamsConfig,
};
Expand Down Expand Up @@ -1814,7 +1814,7 @@ fn test_genesis_validators() -> Result<()> {
.set_port(first_port + 1);
config.ledger.shell.ledger_address.set_port(first_port + 2);
// disable eth full node
config.ledger.ethereum.mode = ethereum::Mode::Off;
config.ledger.ethereum_bridge.mode = ethereum_bridge::ledger::Mode::Off;
config
};

Expand Down
4 changes: 2 additions & 2 deletions tests/src/e2e/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use itertools::{Either, Itertools};
use namada::types::chain::ChainId;
use namada_apps::client::utils;
use namada_apps::config::genesis::genesis_config::{self, GenesisConfig};
use namada_apps::config::{ethereum, Config};
use namada_apps::config::{ethereum_bridge, Config};
use namada_apps::{config, wallet};
use rand::Rng;
use tempfile::{tempdir, TempDir};
Expand Down Expand Up @@ -77,7 +77,7 @@ pub fn update_actor_config<F>(
/// Disable the Ethereum fullnode of `who`.
pub fn disable_eth_fullnode(test: &Test, chain_id: &ChainId, who: &Who) {
update_actor_config(test, chain_id, who, |config| {
config.ledger.ethereum.mode = ethereum::Mode::Off;
config.ledger.ethereum_bridge.mode = ethereum_bridge::ledger::Mode::Off;
});
}

Expand Down