Skip to content

Commit

Permalink
refactor: pass tx_ref as an extrinsic parameter (#4579)
Browse files Browse the repository at this point in the history
* pass tx_ref as an extrinsic parameter
  • Loading branch information
marcellorigotti authored Feb 28, 2024
1 parent a52d476 commit 98937bc
Show file tree
Hide file tree
Showing 14 changed files with 54 additions and 108 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -540,8 +540,8 @@ mod tests {
max_priority_fee_per_gas: None,
contract: H160::from([0; 20]),
gas_limit: None,
tx_hash: Default::default(),
},
transaction_ref: Default::default(),
},
),
&mut store,
Expand Down
28 changes: 13 additions & 15 deletions engine/src/witness/btc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,6 @@ mod btc_chain_tracking;
mod btc_deposits;
pub mod btc_source;

use std::sync::Arc;

use bitcoin::BlockHash;
use cf_chains::btc::{
self, deposit_address::DepositAddress, BitcoinTransactionMetadata, BlockNumber,
CHANGE_ADDRESS_SALT,
};
use cf_primitives::{EpochIndex, NetworkEnvironment};
use futures_core::Future;
use secp256k1::hashes::Hash;
use utilities::task_scope::Scope;

use crate::{
btc::{
retry_rpc::{BtcRetryRpcApi, BtcRetryRpcClient},
Expand All @@ -26,7 +14,16 @@ use crate::{
stream_api::{StreamApi, FINALIZED, UNFINALIZED},
},
};
use bitcoin::BlockHash;
use btc_source::BtcSource;
use cf_chains::btc::{
self, deposit_address::DepositAddress, BitcoinTransactionHash, BlockNumber, CHANGE_ADDRESS_SALT,
};
use cf_primitives::{EpochIndex, NetworkEnvironment};
use futures_core::Future;
use secp256k1::hashes::Hash;
use std::sync::Arc;
use utilities::task_scope::Scope;

use super::common::{
chain_source::{extension::ChainSourceExt, Header},
Expand All @@ -51,15 +48,16 @@ pub async fn process_egress<ProcessCall, ProcessingFut, ExtraInfo, ExtraHistoric

let monitored_tx_hashes = monitored_tx_hashes.iter().map(|(tx_hash, _)| tx_hash);

for (tx_hash, tx) in success_witnesses(monitored_tx_hashes, txs) {
for (tx_hash_bytes_little_endian, tx) in success_witnesses(monitored_tx_hashes, txs) {
process_call(
state_chain_runtime::RuntimeCall::BitcoinBroadcaster(
pallet_cf_broadcast::Call::transaction_succeeded {
tx_out_id: tx_hash,
tx_out_id: tx_hash_bytes_little_endian,
signer_id: DepositAddress::new(epoch.info.0.current, CHANGE_ADDRESS_SALT)
.script_pubkey(),
tx_fee: tx.fee.unwrap_or_default().to_sat(),
tx_metadata: BitcoinTransactionMetadata::new(tx_hash),
tx_metadata: (),
transaction_ref: BitcoinTransactionHash::new(tx_hash_bytes_little_endian),
},
),
epoch.index,
Expand Down
13 changes: 6 additions & 7 deletions engine/src/witness/dot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ mod dot_source;

use cf_chains::dot::{
PolkadotAccountId, PolkadotBalance, PolkadotExtrinsicIndex, PolkadotHash, PolkadotSignature,
PolkadotTransactionId, PolkadotTransactionMetadata, PolkadotUncheckedExtrinsic,
PolkadotTransactionId, PolkadotUncheckedExtrinsic,
};
use cf_primitives::{EpochIndex, PolkadotBlockNumber};
use futures_core::Future;
Expand Down Expand Up @@ -157,12 +157,11 @@ pub async fn process_egress<ProcessCall, ProcessingFut>(
tx_out_id: signature,
signer_id: epoch.info.0,
tx_fee,
tx_metadata: PolkadotTransactionMetadata {
tx_id: PolkadotTransactionId {
block_number: header.index,
extrinsic_index
}
},
tx_metadata: (),
transaction_ref: PolkadotTransactionId {
block_number: header.index,
extrinsic_index
}
}
.into(),
epoch.index,
Expand Down
3 changes: 2 additions & 1 deletion engine/src/witness/eth/key_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ impl<Inner: ChunkedByVault> ChunkedByVaultBuilder<Inner> {
ChainAccount = H160,
TransactionFee = TransactionFee,
TransactionMetadata = EvmTransactionMetadata,
TransactionRef = H256,
>,
ProcessCall: Fn(state_chain_runtime::RuntimeCall, EpochIndex) -> ProcessingFut
+ Send
Expand Down Expand Up @@ -130,7 +131,6 @@ impl<Inner: ChunkedByVault> ChunkedByVaultBuilder<Inner> {
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),
tx_hash: transaction.hash,
};
pallet_cf_broadcast::Call::<
_,
Expand All @@ -143,6 +143,7 @@ impl<Inner: ChunkedByVault> ChunkedByVaultBuilder<Inner> {
signer_id: from,
tx_fee: TransactionFee { effective_gas_price, gas_used },
tx_metadata,
transaction_ref: transaction.hash,
}
.into()
},
Expand Down
1 change: 1 addition & 0 deletions state-chain/cf-integration-tests/src/swapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,7 @@ fn can_resign_failed_ccm() {
gas_used: Default::default()
},
tx_metadata: Default::default(),
transaction_ref: Default::default()
}
)),
<Runtime as Chainflip>::EpochInfo::current_epoch()
Expand Down
13 changes: 6 additions & 7 deletions state-chain/chains/src/benchmarking_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ use crate::address::EncodedAddress;
#[cfg(feature = "runtime-benchmarks")]
use crate::address::ForeignChainAddress;
#[cfg(feature = "runtime-benchmarks")]
use crate::btc::BitcoinTransactionMetadata;
use crate::btc::BitcoinTransactionHash;
#[cfg(feature = "runtime-benchmarks")]
use crate::dot::PolkadotTransactionMetadata;
use crate::dot::PolkadotTransactionId;
#[cfg(feature = "runtime-benchmarks")]
use crate::evm::{EvmFetchId, EvmTransactionMetadata};

Expand Down Expand Up @@ -136,22 +136,21 @@ impl BenchmarkValue for EvmTransactionMetadata {
max_fee_per_gas: Some(U256::zero()),
max_priority_fee_per_gas: Some(U256::zero()),
gas_limit: None,
tx_hash: Default::default(),
}
}
}

#[cfg(feature = "runtime-benchmarks")]
impl BenchmarkValue for PolkadotTransactionMetadata {
impl BenchmarkValue for PolkadotTransactionId {
fn benchmark_value() -> Self {
Self { tx_id: Default::default() }
Self { block_number: 0u32, extrinsic_index: 0u32 }
}
}

#[cfg(feature = "runtime-benchmarks")]
impl BenchmarkValue for BitcoinTransactionMetadata {
impl BenchmarkValue for BitcoinTransactionHash {
fn benchmark_value() -> Self {
Self { tx_hash: Default::default() }
BitcoinTransactionHash::new(Default::default())
}
}

Expand Down
38 changes: 5 additions & 33 deletions state-chain/chains/src/btc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ extern crate alloc;
use self::deposit_address::DepositAddress;
use crate::{
Chain, ChainCrypto, DepositChannel, FeeEstimationApi, FeeRefundCalculator, RetryPolicy,
TransactionMetadata,
};
use alloc::{collections::VecDeque, string::String};
use arrayref::array_ref;
Expand Down Expand Up @@ -238,28 +237,13 @@ impl ConsolidationParameters {
#[derive(
Encode, Decode, TypeInfo, Clone, RuntimeDebug, Default, PartialEq, Eq, Serialize, Deserialize,
)]
pub struct BitcoinTransactionMetadata {
pub tx_hash: Hash,
}
impl BitcoinTransactionMetadata {
pub struct BitcoinTransactionHash(Hash);
impl BitcoinTransactionHash {
/// It creates a tx_hash by reversing the provided hash
/// Btc softwares and explorers display blocks/txs hashes as big endian values, we need to
/// convert it to obtain a valid tx hash
pub fn new(hash: Hash) -> Self {
BitcoinTransactionMetadata { tx_hash: hash.iter().rev().copied().collect_array() }
}
}
impl<C: Chain<TransactionRef = Hash>> TransactionMetadata<C> for BitcoinTransactionMetadata {
fn extract_metadata(_transaction: &<C as Chain>::Transaction) -> Self {
Default::default()
}

fn verify_metadata(&self, _expected_metadata: &Self) -> bool {
true
}

fn get_transaction_ref(&self) -> <C as Chain>::TransactionRef {
self.tx_hash
BitcoinTransactionHash(hash.iter().rev().copied().collect_array())
}
}

Expand All @@ -279,11 +263,11 @@ impl Chain for Bitcoin {
type DepositChannelState = DepositAddress;
type DepositDetails = UtxoId;
type Transaction = BitcoinTransactionData;
type TransactionMetadata = BitcoinTransactionMetadata;
type TransactionMetadata = ();
// There is no need for replay protection on Bitcoin since it is a UTXO chain.
type ReplayProtectionParams = ();
type ReplayProtection = ();
type TransactionRef = Hash;
type TransactionRef = BitcoinTransactionHash;
}

#[derive(Clone, Copy, Encode, Decode, MaxEncodedLen, TypeInfo, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -1464,16 +1448,4 @@ mod test {
assert_eq!(BitcoinRetryPolicy::next_attempt_delay(40), Some(1200));
assert_eq!(BitcoinRetryPolicy::next_attempt_delay(150), Some(1200));
}

#[test]
fn btc_transaction_hash_correctly_derived() {
let tx_out_id: Hash = [
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23,
24, 25, 26, 27, 28, 29, 30, 31,
];
let tx_metadata: BitcoinTransactionMetadata = BitcoinTransactionMetadata::new(tx_out_id);
for (i, item) in tx_out_id.iter().enumerate() {
assert_eq!(*item, tx_metadata.tx_hash[31 - i]);
}
}
}
25 changes: 1 addition & 24 deletions state-chain/chains/src/dot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,29 +314,6 @@ impl FeeEstimationApi<Polkadot> for PolkadotTrackedData {
}
}

#[derive(
Encode, Decode, TypeInfo, Clone, RuntimeDebug, Default, PartialEq, Eq, Serialize, Deserialize,
)]
pub struct PolkadotTransactionMetadata {
pub tx_id: PolkadotTransactionId,
}

impl<C: Chain<TransactionRef = PolkadotTransactionId>> TransactionMetadata<C>
for PolkadotTransactionMetadata
{
fn extract_metadata(_transaction: &<C as Chain>::Transaction) -> Self {
Default::default()
}

fn verify_metadata(&self, _expected_metadata: &Self) -> bool {
true
}

fn get_transaction_ref(&self) -> <C as Chain>::TransactionRef {
self.tx_id.clone()
}
}

#[derive(
Encode, Decode, TypeInfo, Clone, RuntimeDebug, Default, PartialEq, Eq, Serialize, Deserialize,
)]
Expand All @@ -361,7 +338,7 @@ impl Chain for Polkadot {
type DepositChannelState = PolkadotChannelState;
type DepositDetails = ();
type Transaction = PolkadotTransactionData;
type TransactionMetadata = PolkadotTransactionMetadata;
type TransactionMetadata = ();
type ReplayProtectionParams = ResetProxyAccountNonce;
type ReplayProtection = PolkadotReplayProtection;
type TransactionRef = PolkadotTransactionId;
Expand Down
6 changes: 0 additions & 6 deletions state-chain/chains/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,6 @@ pub struct EvmTransactionMetadata {
pub max_priority_fee_per_gas: Option<Uint>,
pub contract: Address,
pub gas_limit: Option<Uint>,
pub tx_hash: H256,
}

impl<C: Chain<Transaction = Transaction, TransactionRef = H256>> TransactionMetadata<C>
Expand All @@ -391,7 +390,6 @@ impl<C: Chain<Transaction = Transaction, TransactionRef = H256>> TransactionMeta
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,
tx_hash: Default::default(),
}
}

Expand All @@ -407,9 +405,6 @@ impl<C: Chain<Transaction = Transaction, TransactionRef = H256>> TransactionMeta
check_optional!(max_priority_fee_per_gas) &&
check_optional!(gas_limit)
}
fn get_transaction_ref(&self) -> C::TransactionRef {
self.tx_hash
}
}

impl Transaction {
Expand Down Expand Up @@ -765,7 +760,6 @@ fn metadata_verification() {
max_priority_fee_per_gas: Some(U256::one()),
contract: Default::default(),
gas_limit: None,
tx_hash: Default::default(),
};

// Exact match.
Expand Down
6 changes: 2 additions & 4 deletions state-chain/chains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ pub trait Chain: Member + Parameter {
+ Default;

/// The type representing the transaction hash for this particular chain
type TransactionRef: Member + Parameter;
type TransactionRef: Member + Parameter + BenchmarkValue;

/// Passed in to construct the replay protection.
type ReplayProtectionParams: Member + Parameter;
Expand Down Expand Up @@ -261,17 +261,15 @@ where
pub trait TransactionMetadata<C: Chain> {
fn extract_metadata(transaction: &C::Transaction) -> Self;
fn verify_metadata(&self, expected_metadata: &Self) -> bool;
fn get_transaction_ref(&self) -> C::TransactionRef;
}

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

/// Contains all the parameters required to fetch incoming transactions on an external chain.
Expand Down
5 changes: 1 addition & 4 deletions state-chain/chains/src/mocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ impl TransactionMetadata<MockEthereum> for MockEthereumTransactionMetadata {
fn verify_metadata(&self, _expected_metadata: &Self) -> bool {
MOCK_VALID_METADATA.with(|cell| *cell.borrow())
}
fn get_transaction_ref(&self) -> <MockEthereum as Chain>::TransactionRef {
Default::default()
}
}

#[cfg(feature = "runtime-benchmarks")]
Expand Down Expand Up @@ -105,7 +102,7 @@ impl Chain for MockEthereum {
type TransactionMetadata = MockEthereumTransactionMetadata;
type ReplayProtectionParams = ();
type ReplayProtection = EvmReplayProtection;
type TransactionRef = ();
type TransactionRef = u32;
}

impl ToHumanreadableAddress for u64 {
Expand Down
1 change: 1 addition & 0 deletions state-chain/pallets/cf-broadcast/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ mod benchmarks {
signer_id,
tx_fee: TransactionFeeFor::<T, I>::benchmark_value(),
tx_metadata: TransactionMetadataFor::<T, I>::benchmark_value(),
transaction_ref: TransactionRefFor::<T, I>::benchmark_value(),
};

#[block]
Expand Down
3 changes: 2 additions & 1 deletion state-chain/pallets/cf-broadcast/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ pub mod pallet {
signer_id: SignerIdFor<T, I>,
tx_fee: TransactionFeeFor<T, I>,
tx_metadata: TransactionMetadataFor<T, I>,
transaction_ref: TransactionRefFor<T, I>,
) -> DispatchResultWithPostInfo {
T::EnsureWitnessed::ensure_origin(origin.clone())?;

Expand Down Expand Up @@ -592,7 +593,7 @@ pub mod pallet {
Self::deposit_event(Event::<T, I>::BroadcastSuccess {
broadcast_id,
transaction_out_id: tx_out_id,
transaction_ref: tx_metadata.get_transaction_ref(),
transaction_ref,
});
Ok(().into())
}
Expand Down
Loading

0 comments on commit 98937bc

Please sign in to comment.