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

feat: verify transaction metadata (#4078)(PRO-819) #4078

Merged
merged 20 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
635b9f5
feat: :sparkles: implemented chain agnostic structure
Janislav Oct 6, 2023
e2c346f
feat: :sparkles: added eth specific implementation
Janislav Oct 6, 2023
4d43e34
refactor: :recycle: changed metadata fields
Janislav Oct 10, 2023
9b88037
feat: :sparkles: added query for get transaction data in CFE
Janislav Oct 10, 2023
a132026
fix: :fire: fixed saving Transaction data at wrong point in time
Janislav Oct 11, 2023
de608f0
feat: :sparkles: added gas_limit
Janislav Oct 11, 2023
3762772
style: :rotating_light: clippy
Janislav Oct 11, 2023
2a3e118
test: :test_tube: added testing
Janislav Oct 12, 2023
271a5c6
refactor: :recycle: renamed event
Janislav Oct 12, 2023
9e23d25
Merge branch 'main' into feature/PRO-819/confirm-evm-transaction
Janislav Oct 17, 2023
4fc2597
refactor: :recycle: implemented requested changes
Janislav Oct 18, 2023
24735d6
refactor: :recycle: moved implementation of TransactionMetaDataHandler
Janislav Oct 19, 2023
0aa337d
chore: removed dead code
Janislav Oct 19, 2023
9c2173e
Merge branch 'main' into feature/PRO-819/confirm-evm-transaction
Janislav Oct 19, 2023
05d8ce4
chore: removed unused code
Janislav Oct 20, 2023
5906341
chore: tidy up
dandanlen Oct 20, 2023
c8517a6
test: test evm metadata verification
dandanlen Oct 20, 2023
34134ef
chore: use () instead of Default::default()
dandanlen Oct 20, 2023
e8705fb
Merge branch 'main' into feature/PRO-819/confirm-evm-transaction
dandanlen Oct 23, 2023
ee0f7f8
Merge branch 'main' into feature/PRO-819/confirm-evm-transaction
Janislav Oct 25, 2023
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
16 changes: 16 additions & 0 deletions engine/src/eth/retry_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ pub trait EthersRetryRpcApi: Clone {
newest_block: BlockNumber,
reward_percentiles: Vec<f64>,
) -> FeeHistory;

async fn get_transaction(&self, tx_hash: H256) -> Transaction;
}

#[async_trait::async_trait]
Expand Down Expand Up @@ -262,6 +264,18 @@ impl EthersRetryRpcApi for EthersRetryRpcClient {
)
.await
}

async fn get_transaction(&self, tx_hash: H256) -> Transaction {
self.rpc_retry_client
.request(
Box::pin(move |client| {
#[allow(clippy::redundant_async_block)]
Box::pin(async move { client.get_transaction(tx_hash).await })
}),
RequestLog::new("get_transaction".to_string(), Some(format!("{tx_hash:?}"))),
)
.await
}
}

#[async_trait::async_trait]
Expand Down Expand Up @@ -360,6 +374,8 @@ pub mod mocks {
newest_block: BlockNumber,
reward_percentiles: Vec<f64>,
) -> FeeHistory;

async fn get_transaction(&self, tx_hash: H256) -> Transaction;
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions engine/src/eth/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ pub trait EthRpcApi: Send {
newest_block: BlockNumber,
reward_percentiles: &[f64],
) -> Result<FeeHistory>;

async fn get_transaction(&self, tx_hash: H256) -> Result<Transaction>;
}

#[async_trait::async_trait]
Expand Down Expand Up @@ -186,6 +188,13 @@ impl EthRpcApi for EthRpcClient {
) -> Result<FeeHistory> {
Ok(self.signer.fee_history(block_count, last_block, reward_percentiles).await?)
}

async fn get_transaction(&self, tx_hash: H256) -> Result<Transaction> {
self.signer
.get_transaction(tx_hash)
.await?
.ok_or_else(|| anyhow!("Getting ETH transaction for tx hash {} returned None", tx_hash))
}
}

/// On each subscription this will create a new WS connection.
Expand Down
1 change: 1 addition & 0 deletions engine/src/witness/btc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub async fn process_egress<ProcessCall, ProcessingFut, ExtraInfo, ExtraHistoric
)
.script_pubkey(),
tx_fee: Default::default(),
tx_metadata: (),
},
),
epoch.index,
Expand Down
1 change: 1 addition & 0 deletions engine/src/witness/dot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ pub async fn process_egress<ProcessCall, ProcessingFut>(
tx_out_id: signature,
signer_id: epoch.info.1,
tx_fee,
tx_metadata: (),
}
.into(),
epoch.index,
Expand Down
19 changes: 16 additions & 3 deletions engine/src/witness/eth/key_manager.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use cf_chains::evm::{EvmCrypto, SchnorrVerificationComponents, TransactionFee};
use cf_chains::evm::{
EvmCrypto, EvmTransactionMetadata, SchnorrVerificationComponents, TransactionFee,
};
use cf_primitives::EpochIndex;
use ethers::{
prelude::abigen,
Expand Down Expand Up @@ -60,6 +62,7 @@ impl<Inner: ChunkedByVault> ChunkedByVaultBuilder<Inner> {
ChainCrypto = EvmCrypto,
ChainAccount = H160,
TransactionFee = TransactionFee,
TransactionMetadata = EvmTransactionMetadata,
>,
ProcessCall: Fn(state_chain_runtime::RuntimeCall, EpochIndex) -> ProcessingFut
+ Send
Expand Down Expand Up @@ -107,8 +110,9 @@ impl<Inner: ChunkedByVault> ChunkedByVaultBuilder<Inner> {
sig_data,
..
}) => {
let TransactionReceipt { gas_used, effective_gas_price, from, .. } =
eth_rpc.transaction_receipt(event.tx_hash).await;
let TransactionReceipt {
gas_used, effective_gas_price, from, to, ..
} = eth_rpc.transaction_receipt(event.tx_hash).await;

let gas_used = gas_used
.ok_or_else(|| {
Expand All @@ -127,6 +131,14 @@ impl<Inner: ChunkedByVault> ChunkedByVaultBuilder<Inner> {
})?
.try_into()
.map_err(anyhow::Error::msg)?;

let transaction = eth_rpc.get_transaction(event.tx_hash).await;
dandanlen marked this conversation as resolved.
Show resolved Hide resolved
let tx_metadata = EvmTransactionMetadata {
contract: to.expect("To have a contract"),
max_fee_per_gas: transaction.max_fee_per_gas,
max_priority_fee_per_gas: transaction.max_priority_fee_per_gas,
gas_limit: Some(transaction.gas),
};
pallet_cf_broadcast::Call::<
_,
<Inner::Chain as PalletInstanceAlias>::Instance,
Expand All @@ -137,6 +149,7 @@ impl<Inner: ChunkedByVault> ChunkedByVaultBuilder<Inner> {
},
signer_id: from,
tx_fee: TransactionFee { effective_gas_price, gas_used },
tx_metadata,
}
.into()
},
Expand Down
1 change: 1 addition & 0 deletions state-chain/chains/src/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ impl Chain for AnyChain {
type DepositChannelState = ();
type DepositDetails = ();
type Transaction = ();
type TransactionMetadata = ();
type ReplayProtectionParams = ();
type ReplayProtection = ();
}
Expand Down
15 changes: 15 additions & 0 deletions state-chain/chains/src/benchmarking_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ use cf_primitives::{
chains::assets::{btc, dot, eth},
Asset,
};
use ethereum_types::{H160, U256};

#[cfg(feature = "runtime-benchmarks")]
use crate::address::EncodedAddress;
#[cfg(feature = "runtime-benchmarks")]
use crate::address::ForeignChainAddress;
#[cfg(feature = "runtime-benchmarks")]
use crate::evm::EvmFetchId;
use crate::evm::EvmTransactionMetadata;

/// Ensure type specifies a value to be used for benchmarking purposes.
pub trait BenchmarkValue {
Expand Down Expand Up @@ -120,6 +122,19 @@ impl BenchmarkValueExtended for () {
Default::default()
}
}

#[cfg(feature = "runtime-benchmarks")]
impl BenchmarkValue for EvmTransactionMetadata {
fn benchmark_value() -> Self {
Self {
contract: H160::zero(),
max_fee_per_gas: Some(U256::zero()),
max_priority_fee_per_gas: Some(U256::zero()),
gas_limit: None,
}
}
}

impl_default_benchmark_value!(());
impl_default_benchmark_value!(u32);
impl_default_benchmark_value!(u64);
1 change: 1 addition & 0 deletions state-chain/chains/src/btc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ impl Chain for Bitcoin {
type DepositChannelState = DepositAddress;
type DepositDetails = UtxoId;
type Transaction = BitcoinTransactionData;
type TransactionMetadata = ();
// There is no need for replay protection on Bitcoin since it is a UTXO chain.
type ReplayProtectionParams = ();
type ReplayProtection = ();
Expand Down
1 change: 1 addition & 0 deletions state-chain/chains/src/dot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ impl Chain for Polkadot {
type DepositChannelState = PolkadotChannelState;
type DepositDetails = ();
type Transaction = PolkadotTransactionData;
type TransactionMetadata = ();
type ReplayProtectionParams = ResetProxyAccountNonce;
type ReplayProtection = PolkadotReplayProtection;
}
Expand Down
3 changes: 2 additions & 1 deletion state-chain/chains/src/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub mod benchmarking;
pub mod deposit_address;

use crate::{
evm::{DeploymentStatus, EvmFetchId, Transaction},
evm::{DeploymentStatus, EvmFetchId, EvmTransactionMetadata, Transaction},
*,
};
use cf_primitives::chains::assets;
Expand Down Expand Up @@ -43,6 +43,7 @@ impl Chain for Ethereum {
type DepositChannelState = DeploymentStatus;
type DepositDetails = ();
type Transaction = Transaction;
type TransactionMetadata = EvmTransactionMetadata;
type ReplayProtectionParams = Self::ChainAccount;
type ReplayProtection = EvmReplayProtection;
}
Expand Down
80 changes: 80 additions & 0 deletions state-chain/chains/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,40 @@ pub struct Transaction {
pub data: Vec<u8>,
}

#[derive(
Encode, Decode, TypeInfo, Clone, RuntimeDebug, Default, PartialEq, Eq, Serialize, Deserialize,
)]
pub struct EvmTransactionMetadata {
pub max_fee_per_gas: Option<Uint>,
pub max_priority_fee_per_gas: Option<Uint>,
pub contract: Address,
pub gas_limit: Option<Uint>,
}

impl<C: Chain<Transaction = Transaction>> TransactionMetadata<C> for EvmTransactionMetadata {
fn extract_metadata(transaction: &<C as Chain>::Transaction) -> Self {
Self {
contract: transaction.contract,
max_fee_per_gas: transaction.max_fee_per_gas,
max_priority_fee_per_gas: transaction.max_priority_fee_per_gas,
gas_limit: transaction.gas_limit,
}
}

fn verify_metadata(&self, expected_metadata: &Self) -> bool {
macro_rules! check_optional {
($field:ident) => {
(expected_metadata.$field.is_none() || expected_metadata.$field == self.$field)
};
}

self.contract == expected_metadata.contract &&
check_optional!(max_fee_per_gas) &&
check_optional!(max_priority_fee_per_gas) &&
check_optional!(gas_limit)
}
}

impl Transaction {
fn check_contract(
&self,
Expand Down Expand Up @@ -700,3 +734,49 @@ mod verification_tests {
);
}
}

#[test]
fn metadata_verification() {
let submitted_metadata = EvmTransactionMetadata {
max_fee_per_gas: None,
max_priority_fee_per_gas: Some(U256::one()),
contract: Default::default(),
gas_limit: None,
};

// Exact match.
assert!(<EvmTransactionMetadata as TransactionMetadata<Ethereum>>::verify_metadata(
&submitted_metadata,
&submitted_metadata
));

// If we don't expect a value, it's ok if it's set.
assert!(<EvmTransactionMetadata as TransactionMetadata<Ethereum>>::verify_metadata(
&submitted_metadata,
&EvmTransactionMetadata { max_priority_fee_per_gas: None, ..submitted_metadata }
));

// If we expect something else it fails.
assert!(!<EvmTransactionMetadata as TransactionMetadata<Ethereum>>::verify_metadata(
&submitted_metadata,
&EvmTransactionMetadata {
max_priority_fee_per_gas: Some(U256::zero()),
..submitted_metadata
}
));

// If we witness `None` instead of `Some`, it fails.
assert!(!<EvmTransactionMetadata as TransactionMetadata<Ethereum>>::verify_metadata(
&submitted_metadata,
&EvmTransactionMetadata { max_fee_per_gas: Some(U256::zero()), ..submitted_metadata }
));

// Wrong contract address.
assert!(!<EvmTransactionMetadata as TransactionMetadata<Ethereum>>::verify_metadata(
&submitted_metadata,
&EvmTransactionMetadata {
contract: ethereum_types::H160::repeat_byte(1u8),
..submitted_metadata
}
));
}
20 changes: 20 additions & 0 deletions state-chain/chains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ pub trait Chain: Member + Parameter {
type DepositDetails: Member + Parameter + BenchmarkValue;

type Transaction: Member + Parameter + BenchmarkValue + FeeRefundCalculator<Self>;

type TransactionMetadata: Member
+ Parameter
+ TransactionMetadata<Self>
+ BenchmarkValue
+ Default;
/// Passed in to construct the replay protection.
type ReplayProtectionParams: Member + Parameter;
type ReplayProtection: Member + Parameter;
Expand Down Expand Up @@ -254,6 +260,20 @@ where
}
}

pub trait TransactionMetadata<C: Chain> {
fn extract_metadata(transaction: &C::Transaction) -> Self;
fn verify_metadata(&self, expected_metadata: &Self) -> bool;
}

impl<C: Chain> TransactionMetadata<C> for () {
fn extract_metadata(_transaction: &C::Transaction) -> Self {
Default::default()
}
fn verify_metadata(&self, _expected_metadata: &Self) -> bool {
true
}
}

/// Contains all the parameters required to fetch incoming transactions on an external chain.
#[derive(RuntimeDebug, Copy, Clone, PartialEq, Eq, Encode, Decode, MaxEncodedLen, TypeInfo)]
pub struct FetchAssetParams<C: Chain> {
Expand Down
30 changes: 30 additions & 0 deletions state-chain/chains/src/mocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ thread_local! {
static MOCK_KEY_HANDOVER_IS_REQUIRED: RefCell<bool> = RefCell::new(true);
static MOCK_OPTIMISTIC_ACTIVATION: RefCell<bool> = RefCell::new(false);
static MOCK_SIGN_WITH_SPECIFIC_KEY: RefCell<bool> = RefCell::new(false);
static MOCK_VALID_METADATA: RefCell<bool> = RefCell::new(true);
}

pub struct MockKeyHandoverIsRequired;
Expand Down Expand Up @@ -62,6 +63,31 @@ impl Get<bool> for MockFixedKeySigningRequests {
}
}

#[derive(Debug, Clone, Default, PartialEq, Eq, Encode, Decode, TypeInfo)]
pub struct MockEthereumTransactionMetadata;

impl TransactionMetadata<MockEthereum> for MockEthereumTransactionMetadata {
fn extract_metadata(_transaction: &<MockEthereum as Chain>::Transaction) -> Self {
Default::default()
}

fn verify_metadata(&self, _expected_metadata: &Self) -> bool {
MOCK_VALID_METADATA.with(|cell| *cell.borrow())
}
}

impl BenchmarkValue for MockEthereumTransactionMetadata {
fn benchmark_value() -> Self {
Default::default()
}
}

impl MockEthereumTransactionMetadata {
pub fn set_validity(valid: bool) {
MOCK_VALID_METADATA.with(|cell| *cell.borrow_mut() = valid);
}
}

// Chain implementation used for testing.
impl Chain for MockEthereum {
const NAME: &'static str = "MockEthereum";
Expand All @@ -78,6 +104,7 @@ impl Chain for MockEthereum {
type DepositChannelState = MockLifecycleHooks;
type DepositDetails = [u8; 4];
type Transaction = MockTransaction;
type TransactionMetadata = MockEthereumTransactionMetadata;
type ReplayProtectionParams = ();
type ReplayProtection = EvmReplayProtection;
}
Expand Down Expand Up @@ -255,6 +282,9 @@ pub const MOCK_TRANSACTION_OUT_ID: [u8; 4] = [0xbc; 4];
pub const ETH_TX_FEE: <MockEthereum as Chain>::TransactionFee =
TransactionFee { effective_gas_price: 200, gas_used: 100 };

pub const MOCK_TX_METADATA: <MockEthereum as Chain>::TransactionMetadata =
MockEthereumTransactionMetadata;

#[derive(Encode, Decode, TypeInfo, CloneNoBound, DebugNoBound, PartialEqNoBound, EqNoBound)]
#[scale_info(skip_type_params(C))]
pub struct MockApiCall<C: ChainCrypto> {
Expand Down
1 change: 1 addition & 0 deletions state-chain/chains/src/none.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ impl Chain for NoneChain {
type DepositChannelState = ();
type DepositDetails = ();
type Transaction = ();
type TransactionMetadata = ();
type ReplayProtectionParams = ();
type ReplayProtection = ();
}
Expand Down
Loading