diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 15836ddaec..345639bcde 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -82,11 +82,9 @@ pub enum BoostStatus { #[derive(Clone, Debug, PartialEq, Eq, Encode, Decode, TypeInfo, Default)] pub enum TaintedTransactionStatus { - /// Transaction was boosted, can't be rejected. - Boosted, /// Transaction was prewitnessed but not boosted due to being reported. Prewitnessed, - /// Transaction has been reported but not neither prewitnessed nor boosted. + /// Transaction has been reported but was not prewitnessed. #[default] Unseen, } @@ -849,7 +847,7 @@ pub mod pallet { Ok(()) }, _ => { - // Don't apply the mutation. We expect the pre-witnessed/boosted + // Don't apply the mutation. We expect the pre-witnessed // transaction to eventually be fully witnessed. Err(()) }, @@ -1725,16 +1723,12 @@ impl, I: 'static> Pallet { *status = TaintedTransactionStatus::Prewitnessed; true }, + // Pre-witnessing twice is unlikely but possible. Either way we don't want + // to change the status and we don't want to allow boosting. + Some(TaintedTransactionStatus::Prewitnessed) => true, // Transaction has not been reported, mark it as boosted to prevent further // reports. - None => { - let _ = opt.insert(TaintedTransactionStatus::Boosted); - false - }, - // Pre-witnessing twice or pre-witnessing after boosting is unlikely but - // possible. Either way we don't want to change the status. - Some(TaintedTransactionStatus::Prewitnessed) | - Some(TaintedTransactionStatus::Boosted) => true, + None => false, } }) { continue; @@ -1963,8 +1957,8 @@ impl, I: 'static> Pallet { let channel_owner = deposit_channel_details.owner.clone(); if let Some(tx_id) = deposit_details.deposit_id() { - if matches!(TaintedTransactions::::take(&channel_owner, &tx_id), - Some(status) if status != TaintedTransactionStatus::Boosted) + if TaintedTransactions::::take(&channel_owner, &tx_id).is_some() && + !matches!(deposit_channel_details.boost_status, BoostStatus::Boosted { .. }) { let refund_address = match deposit_channel_details.action.clone() { ChannelAction::Swap { refund_params, .. } => refund_params diff --git a/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs b/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs index 4daf026526..123389d33b 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs @@ -168,10 +168,8 @@ fn finalize_boosted_tx_if_tainted_after_prewitness() { 10, ); - assert_noop!( - IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), tx_id,), - crate::Error::::TransactionAlreadyPrewitnessed - ); + // It's possible to report the tx, but reporting will have no effect. + assert_ok!(IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), tx_id,),); assert_ok!(IngressEgress::process_single_deposit( address, @@ -319,7 +317,6 @@ fn can_not_report_transaction_after_witnessing() { let unreported = Hash::random(); let unseen = Hash::random(); let prewitnessed = Hash::random(); - let boosted = Hash::random(); TaintedTransactions::::insert(BROKER, unseen, TaintedTransactionStatus::Unseen); TaintedTransactions::::insert( @@ -327,7 +324,6 @@ fn can_not_report_transaction_after_witnessing() { prewitnessed, TaintedTransactionStatus::Prewitnessed, ); - TaintedTransactions::::insert(BROKER, boosted, TaintedTransactionStatus::Boosted); assert_ok!(IngressEgress::mark_transaction_as_tainted( OriginTrait::signed(BROKER), @@ -340,10 +336,6 @@ fn can_not_report_transaction_after_witnessing() { IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), prewitnessed,), crate::Error::::TransactionAlreadyPrewitnessed ); - assert_noop!( - IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), boosted,), - crate::Error::::TransactionAlreadyPrewitnessed - ); }); } @@ -373,3 +365,50 @@ fn send_funds_back_after_they_have_been_rejected() { ); }); } + +#[test] +fn can_report_between_prewitness_and_witness_if_tx_was_not_boosted() { + new_test_ext().execute_with(|| { + let tx_id = Hash::random(); + let deposit_details = helpers::generate_btc_deposit(tx_id); + + assert_ok!(>::register_as_broker( + &BROKER, + )); + + let (_id, address, ..) = IngressEgress::request_liquidity_deposit_address( + BROKER, + btc::Asset::Btc, + 0, + ForeignChainAddress::Btc(ScriptPubkey::P2SH(DEFAULT_BTC_ADDRESS)), + ) + .unwrap(); + + let deposit_address = match address { + ForeignChainAddress::Btc(script_pubkey) => script_pubkey, + _ => unreachable!(), + }; + + let deposit_witnesses = vec![DepositWitness { + deposit_address, + asset: btc::Asset::Btc, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details, + }]; + + assert_ok!(IngressEgress::add_prewitnessed_deposits(deposit_witnesses.clone(), 10,)); + assert_ok!(IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), tx_id,)); + assert_ok!(IngressEgress::process_deposit_witnesses(deposit_witnesses, 10,)); + + assert_has_matching_event!( + Test, + RuntimeEvent::IngressEgress(crate::Event::DepositIgnored { + deposit_address: _, + asset: btc::Asset::Btc, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details: _, + reason: DepositIgnoredReason::TransactionTainted, + }) + ); + }); +}