Skip to content

Commit

Permalink
feature/PRO-1712/reject-tx (#5332)
Browse files Browse the repository at this point in the history
* feat: Handling of tainted transactions

- Extended DepositChannelDetails with owner field
- Added extrinsic to mark transaction as tainted
- Handling deposit and save details for a refund if tx is tainted

* tests: Added tests

- Added tests to verify that tainted transactions can get detected for all possible swap types
- Added tests  to check that txs marked by other brokers are getting ignored

* chore: Extended LpRegistration trait + Added to Ingress/Egress config

* feat: Getting LP refund address

* feature: ensure by lp and broker + added more tests

* refactor: Don't error if tx is tainted

* refactor: Using DoubleMap instead of Map

* refactor: Using BadOrigin + Added unit test

* refactor: Inline code + Add Deposit Witness to struct

* refactor: Extended lp deposit with refund address

- Taking refund address if we open a deposit channel as lp
- Extended ChannelAction to take an optional refund address
- Removed all dependencies regarding the LpRegistration trait (added last commit)
- Refactored tests, benchmarks, etc

* feature: Added benchmark

* chore: Changes to benchmark

* chore: generated mock weights

* chore: Added POC for BTC swap rejecting.

* chore: only allow mark transactions for BTC

* feature: expire tainted transaction

* test: refactored tests

* chore: Extended RejectCall with DepositWitness

* Revert "chore: Extended RejectCall with DepositWitness"

This reverts commit 67873ff.

* chore: added draft for eth refund implementation

* chore: Removed unused events

* chore: Ensure only by broker

* chore: removed broker from tainted tx struct

* chore: Only clone owner

* chore: Moved tx tainted check

* feature: Added migration for DepositChannelLookup

* refactor: Changed data structure + fixed migrations

* chore: Handle LP refund address as requirement

* chore: Made clippy happy 🙂

* chore: don't manipulate storage in place in iteration 🙅‍♂️

* test: Added migration test 🧪

* chore: changed pallet storage version 📀

* chore: bumped pallet storage version (again)

* chore: using ScheduledTxForReject for refunding

* refactor: Changed accounting of expired transactions

* refactor: using translate for migration

* refactor: using append, refactored test

* feature: Added handling of boost channels

* feature: Marking txs when prewitness and reject when we process the depo

* feat: pre-witnessed rejection handling

* chore: Fixed logic + added tests

* tests: Refactor/Rearranged tests

* chore: Using SECONDS_PER_BLOCK instead of static block seconds

* chore: Addressed comments

* chore: Fixed clippy in CI

* chore: update comments

* fix: don't allow report overwrite

* chore: Renamed event

* feat: improvements:

- mark boosted transactions as boosted instead of using channel status
- allow pallet config instead of relying on chain
- add event for tx reports
- only allow reporting of unseen transactions
- add doc comments
- renaming of types/events
- remove unused error

* chore: removed not needed RefundParameters, added test

* refactor: Rejecting

- Changed trait
- Refactor AllBatch build
- LogOrPanic if reject failed and safe details

* chore: fix compiler errors

* chore: subtract fees

* fix: using deposit address

* fix: addressed problems

- Introduced NoChangeTransfer ApiCall
- Fire event when tx was broadcasted
- Handle on_broadcast_ready with no-op

* chore: small improvements

- Removed not needed derives
- Clippy

* wip: change depositDetails for btc to utxo

* Revert "wip: change depositDetails for btc to utxo"

This reverts commit 55de221.

* refactor: added current status of tx_id refactor

* tests: Moved screening feature tests to own file and test against btc

* refactor:

- Fire event if we can not reject tx
- Split amount and fees in trait
- Fix UTXO construct

* chore: rearranged imports

* refactors:

- use Utxo and remove BtcDepositDetails
- use saturating_sub for egress fee subtraction
- use ChainAccount instead of ForeignChainAddress in RejectCall
- simplify definitions of RejectCall using default method

---------

Co-authored-by: Daniel <daniel@chainflip.io>
Co-authored-by: Maxim Shishmarev <maxim@chainflip.io>
  • Loading branch information
3 people authored Oct 29, 2024
1 parent 92436ff commit 079a149
Show file tree
Hide file tree
Showing 28 changed files with 865 additions and 606 deletions.
351 changes: 183 additions & 168 deletions Cargo.lock

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ impl From<DepositInfo<Bitcoin>> for WitnessInformation {
amount: value.amount.into(),
asset: value.asset.into(),
deposit_details: Some(DepositDetails::Bitcoin {
tx_id: value.deposit_details.utxo_id.tx_id,
vout: value.deposit_details.utxo_id.vout,
tx_id: value.deposit_details.id.tx_id,
vout: value.deposit_details.id.vout,
}),
}
}
Expand Down
10 changes: 6 additions & 4 deletions engine/src/witness/btc/deposits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
use bitcoin::{hashes::Hash, BlockHash};
use cf_chains::{
assets::btc,
btc::{deposit_address::DepositAddress, BtcDepositDetails, UtxoId},
btc::{deposit_address::DepositAddress, Utxo, UtxoId},
Bitcoin,
};

Expand Down Expand Up @@ -115,12 +115,14 @@ fn deposit_witnesses(
.filter(|(_vout, tx_out)| tx_out.value.to_sat() > 0)
.filter_map(|(vout, tx_out)| {
deposit_addresses.get(tx_out.script_pubkey.as_bytes()).map(|deposit_address| {
let amount = tx_out.value.to_sat();
DepositWitness::<Bitcoin> {
deposit_address: deposit_address.script_pubkey(),
asset: btc::Asset::Btc,
amount: tx_out.value.to_sat(),
deposit_details: BtcDepositDetails {
utxo_id: UtxoId { tx_id: tx.txid.to_byte_array().into(), vout },
amount,
deposit_details: Utxo {
id: UtxoId { tx_id: tx.txid.to_byte_array().into(), vout },
amount,
deposit_address: deposit_address.clone(),
},
}
Expand Down
14 changes: 8 additions & 6 deletions engine/src/witness/btc/smart_contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use cf_amm::common::{bounded_sqrt_price, sqrt_price_to_price};
use cf_chains::{
assets::btc::Asset as BtcAsset,
btc::{
deposit_address::DepositAddress, smart_contract_encoding::UtxoEncodedData,
BtcDepositDetails, ScriptPubkey, UtxoId,
deposit_address::DepositAddress, smart_contract_encoding::UtxoEncodedData, ScriptPubkey,
Utxo, UtxoId,
},
ChannelRefundParameters, ForeignChainAddress,
};
Expand Down Expand Up @@ -134,9 +134,10 @@ pub fn try_extract_contract_call(
deposit_amount,
destination_address: data.output_address,
tx_hash: tx_id,
deposit_details: Box::new(BtcDepositDetails {
deposit_details: Box::new(Utxo {
// we require the deposit to be the first UTXO
utxo_id: UtxoId { tx_id: tx_id.into(), vout: 0 },
id: UtxoId { tx_id: tx_id.into(), vout: 0 },
amount: deposit_amount,
deposit_address: vault_address.clone(),
}),
deposit_metadata: None, // No ccm for BTC (yet?)
Expand Down Expand Up @@ -274,8 +275,9 @@ mod tests {
deposit_amount: DEPOSIT_AMOUNT,
destination_address: MOCK_SWAP_PARAMS.output_address.clone(),
tx_hash: tx.hash.to_byte_array(),
deposit_details: Box::new(BtcDepositDetails {
utxo_id: UtxoId { tx_id: tx.txid.to_byte_array().into(), vout: 0 },
deposit_details: Box::new(Utxo {
id: UtxoId { tx_id: tx.txid.to_byte_array().into(), vout: 0 },
amount: DEPOSIT_AMOUNT,
deposit_address: vault_deposit_address,
}),
broker_fees: Default::default(),
Expand Down
12 changes: 6 additions & 6 deletions state-chain/cf-integration-tests/src/broadcasting.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::*;
use cf_chains::{
btc::{deposit_address::DepositAddress, ScriptPubkey},
btc::{deposit_address::DepositAddress, ScriptPubkey, Utxo},
AllBatch, Bitcoin, ForeignChain, TransferAssetParams,
};
use cf_primitives::{chains::assets::btc, AuthorityCount, BroadcastId};
Expand All @@ -26,11 +26,11 @@ fn bitcoin_broadcast_delay_works() {
let epoch = Validator::epoch_index();
let bitcoin_agg_key = BitcoinThresholdSigner::keys(epoch).unwrap().current;
let egress_id = (ForeignChain::Bitcoin, 1u64);
Environment::add_bitcoin_utxo_to_list(
1_000_000_000_000u64,
Default::default(),
DepositAddress::new(bitcoin_agg_key, 0u32),
);
Environment::add_bitcoin_utxo_to_list(Utxo {
id: Default::default(),
amount: 1_000_000_000_000u64,
deposit_address: DepositAddress::new(bitcoin_agg_key, 0u32),
});

// Cause bitcoin vault to rotate - but stop the broadcasting.
let (bitcoin_call, _egress_ids) = AllBatch::<Bitcoin>::new_unsigned(
Expand Down
2 changes: 1 addition & 1 deletion state-chain/cf-integration-tests/src/new_epoch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ fn utxo(amount: BtcAmount, salt: u32, pub_key: Option<[u8; 32]>) -> Utxo {
}

fn add_utxo_amount(utxo: Utxo) {
Environment::add_bitcoin_utxo_to_list(utxo.amount, utxo.id, utxo.deposit_address);
Environment::add_bitcoin_utxo_to_list(utxo);
}

#[test]
Expand Down
4 changes: 3 additions & 1 deletion state-chain/chains/src/any.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
address::{ForeignChainAddress, IntoForeignChainAddress},
none::NoneChainCrypto,
Chain, FeeRefundCalculator,
Chain, DepositDetailsToTransactionInId, FeeRefundCalculator,
};
use codec::{FullCodec, MaxEncodedLen};
use frame_support::Parameter;
Expand Down Expand Up @@ -52,3 +52,5 @@ impl IntoForeignChainAddress<AnyChain> for ForeignChainAddress {
address
}
}

impl DepositDetailsToTransactionInId<NoneChainCrypto> for () {}
5 changes: 5 additions & 0 deletions state-chain/chains/src/arb/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,11 @@ impl<E> From<EvmTransactionBuilder<transfer_fallback::TransferFallback>> for Arb
}
}

impl<E> RejectCall<Arbitrum> for ArbitrumApi<E> where
E: EvmEnvironmentProvider<Arbitrum> + ReplayProtectionProvider<Arbitrum>
{
}

macro_rules! map_over_api_variants {
( $self:expr, $var:pat_param, $var_method:expr $(,)* ) => {
match $self {
Expand Down
9 changes: 5 additions & 4 deletions state-chain/chains/src/benchmarking_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use sp_std::vec;
#[cfg(feature = "runtime-benchmarks")]
use crate::{
address::{EncodedAddress, ForeignChainAddress},
btc::{BtcDepositDetails, UtxoId},
btc::{Utxo, UtxoId},
dot::PolkadotTransactionId,
evm::{DepositDetails, EvmFetchId, EvmTransactionMetadata},
};
Expand Down Expand Up @@ -225,10 +225,11 @@ impl BenchmarkValue for DepositDetails {
}

#[cfg(feature = "runtime-benchmarks")]
impl BenchmarkValue for BtcDepositDetails {
impl BenchmarkValue for Utxo {
fn benchmark_value() -> Self {
BtcDepositDetails {
utxo_id: UtxoId::benchmark_value(),
Utxo {
id: UtxoId::benchmark_value(),
amount: 10_000_000,
deposit_address: crate::btc::deposit_address::DepositAddress::new([0; 32], 0),
}
}
Expand Down
18 changes: 9 additions & 9 deletions state-chain/chains/src/btc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ pub mod utxo_selection;
extern crate alloc;
use self::deposit_address::DepositAddress;
use crate::{
benchmarking_value::BenchmarkValue, Chain, ChainCrypto, DepositChannel, FeeEstimationApi,
FeeRefundCalculator, RetryPolicy,
benchmarking_value::BenchmarkValue, Chain, ChainCrypto, DepositChannel,
DepositDetailsToTransactionInId, FeeEstimationApi, FeeRefundCalculator, RetryPolicy,
};
use alloc::{collections::VecDeque, string::String};
use arrayref::array_ref;
Expand Down Expand Up @@ -227,12 +227,6 @@ impl BitcoinFeeInfo {
}
}

#[derive(Encode, Decode, TypeInfo, Clone, RuntimeDebug, PartialEq, Eq, MaxEncodedLen)]
pub struct BtcDepositDetails {
pub utxo_id: UtxoId,
pub deposit_address: DepositAddress,
}

impl Chain for Bitcoin {
const NAME: &'static str = "Bitcoin";
const GAS_ASSET: Self::ChainAsset = assets::btc::Asset::Btc;
Expand All @@ -250,7 +244,7 @@ impl Chain for Bitcoin {
type ChainAccount = ScriptPubkey;
type DepositFetchId = BitcoinFetchId;
type DepositChannelState = DepositAddress;
type DepositDetails = BtcDepositDetails;
type DepositDetails = Utxo;
type Transaction = BitcoinTransactionData;
type TransactionMetadata = ();
type TransactionRef = Hash;
Expand Down Expand Up @@ -406,6 +400,12 @@ impl Utxo {
}
}

impl DepositDetailsToTransactionInId<BitcoinCrypto> for Utxo {
fn deposit_id(&self) -> Option<Hash> {
Some(self.id.tx_id)
}
}

#[derive(Encode, Decode, TypeInfo, MaxEncodedLen, Clone, RuntimeDebug, PartialEq, Eq)]
pub struct BitcoinOutput {
pub amount: u64,
Expand Down
35 changes: 31 additions & 4 deletions state-chain/chains/src/btc/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use super::{
deposit_address::DepositAddress, AggKey, Bitcoin, BitcoinCrypto, BitcoinOutput, BtcAmount,
Utxo, BITCOIN_DUST_LIMIT, CHANGE_ADDRESS_SALT,
};
use crate::*;
use crate::{btc::BitcoinTransaction, *};
use frame_support::{CloneNoBound, DebugNoBound, EqNoBound, Never, PartialEqNoBound};
use sp_std::marker::PhantomData;

Expand All @@ -13,6 +13,7 @@ use sp_std::marker::PhantomData;
#[allow(clippy::large_enum_variant)]
pub enum BitcoinApi<Environment: 'static> {
BatchTransfer(batch_transfer::BatchTransfer),
NoChangeTransfer(BitcoinTransaction),
#[doc(hidden)]
#[codec(skip)]
_Phantom(PhantomData<Environment>, Never),
Expand Down Expand Up @@ -146,6 +147,25 @@ impl<E: ReplayProtectionProvider<Bitcoin>> ExecutexSwapAndCall<Bitcoin> for Bitc
}
}

impl<E: ReplayProtectionProvider<Bitcoin>> RejectCall<Bitcoin> for BitcoinApi<E>
where
E: ChainEnvironment<UtxoSelectionType, SelectedUtxosAndChangeAmount>
+ ChainEnvironment<(), AggKey>,
{
fn new_unsigned(
deposit_details: <Bitcoin as Chain>::DepositDetails,
refund_address: <Bitcoin as Chain>::ChainAccount,
refund_amount: <Bitcoin as Chain>::ChainAmount,
) -> Result<Self, RejectError> {
let agg_key = <E as ChainEnvironment<(), AggKey>>::lookup(()).ok_or(RejectError::Other)?;
Ok(Self::NoChangeTransfer(BitcoinTransaction::create_new_unsigned(
&agg_key,
vec![deposit_details],
vec![BitcoinOutput { amount: refund_amount, script_pubkey: refund_address }],
)))
}
}

// transfer_fallback is unsupported for Bitcoin.
impl<E: ReplayProtectionProvider<Bitcoin>> TransferFallback<Bitcoin> for BitcoinApi<E> {
fn new_unsigned(
Expand All @@ -159,7 +179,7 @@ impl<E> ApiCall<BitcoinCrypto> for BitcoinApi<E> {
fn threshold_signature_payload(&self) -> <BitcoinCrypto as ChainCrypto>::Payload {
match self {
BitcoinApi::BatchTransfer(tx) => tx.threshold_signature_payload(),

BitcoinApi::NoChangeTransfer(tx) => tx.get_signing_payloads(),
BitcoinApi::_Phantom(..) => unreachable!(),
}
}
Expand All @@ -171,29 +191,34 @@ impl<E> ApiCall<BitcoinCrypto> for BitcoinApi<E> {
) -> Self {
match self {
BitcoinApi::BatchTransfer(call) => call.signed(threshold_signature, signer).into(),
BitcoinApi::NoChangeTransfer(mut tx) => {
tx.add_signer_and_signatures(signer, threshold_signature.clone());
Self::NoChangeTransfer(tx)
},
BitcoinApi::_Phantom(..) => unreachable!(),
}
}

fn chain_encoded(&self) -> Vec<u8> {
match self {
BitcoinApi::BatchTransfer(call) => call.chain_encoded(),

BitcoinApi::NoChangeTransfer(call) => call.clone().finalize(),
BitcoinApi::_Phantom(..) => unreachable!(),
}
}

fn is_signed(&self) -> bool {
match self {
BitcoinApi::BatchTransfer(call) => call.is_signed(),

BitcoinApi::NoChangeTransfer(call) => call.is_signed(),
BitcoinApi::_Phantom(..) => unreachable!(),
}
}

fn transaction_out_id(&self) -> <BitcoinCrypto as ChainCrypto>::TransactionOutId {
match self {
BitcoinApi::BatchTransfer(call) => call.transaction_out_id(),
BitcoinApi::NoChangeTransfer(call) => call.txid(),
BitcoinApi::_Phantom(..) => unreachable!(),
}
}
Expand All @@ -205,6 +230,8 @@ impl<E> ApiCall<BitcoinCrypto> for BitcoinApi<E> {
fn signer(&self) -> Option<<BitcoinCrypto as ChainCrypto>::AggKey> {
match self {
BitcoinApi::BatchTransfer(call) => call.signer(),
BitcoinApi::NoChangeTransfer(call) =>
call.signer_and_signatures.as_ref().map(|(signer, _)| (*signer)),
BitcoinApi::_Phantom(..) => unreachable!(),
}
}
Expand Down
2 changes: 2 additions & 0 deletions state-chain/chains/src/dot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,8 @@ pub enum PolkadotRuntimeCall {
#[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo)]
pub enum SystemCall {}

impl DepositDetailsToTransactionInId<PolkadotCrypto> for u32 {}

#[allow(non_camel_case_types)]
#[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo)]
pub enum BalancesCall {
Expand Down
5 changes: 5 additions & 0 deletions state-chain/chains/src/dot/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ where
}
}

impl<E> RejectCall<Polkadot> for PolkadotApi<E> where
E: PolkadotEnvironment + ReplayProtectionProvider<Polkadot>
{
}

#[macro_export]
macro_rules! map_over_api_variants {
( $self:expr, $var:pat_param, $var_method:expr $(,)* ) => {
Expand Down
5 changes: 5 additions & 0 deletions state-chain/chains/src/eth/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,11 @@ where
}
}

impl<E> RejectCall<Ethereum> for EthereumApi<E> where
E: EvmEnvironmentProvider<Ethereum> + ReplayProtectionProvider<Ethereum>
{
}

impl<E> From<EvmTransactionBuilder<set_agg_key_with_agg_key::SetAggKeyWithAggKey>>
for EthereumApi<E>
{
Expand Down
8 changes: 8 additions & 0 deletions state-chain/chains/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ use serde::{Deserialize, Serialize};
use sp_core::ConstBool;
use sp_std::{convert::TryFrom, str, vec};

use crate::DepositDetailsToTransactionInId;

#[derive(Clone, Debug, PartialEq, Eq, Encode, Decode, TypeInfo, Default)]
pub struct DepositDetails {
// In the case of EVM Native Deposits (ETH or arbETH), because we need to detect ingresses by
Expand Down Expand Up @@ -650,6 +652,12 @@ pub struct TransactionFee {
pub gas_used: u128,
}

impl DepositDetailsToTransactionInId<EvmCrypto> for DepositDetails {
fn deposit_id(&self) -> Option<H256> {
None
}
}

#[cfg(test)]
pub(crate) mod tests {
use super::*;
Expand Down
Loading

0 comments on commit 079a149

Please sign in to comment.