diff --git a/bouncer/tests/evm_deposits.ts b/bouncer/tests/evm_deposits.ts index 8d9dacfe4a..c787e6b320 100644 --- a/bouncer/tests/evm_deposits.ts +++ b/bouncer/tests/evm_deposits.ts @@ -186,6 +186,8 @@ async function testDoubleDeposit(sourceAsset: Asset, destAsset: Asset, _testCont } export async function testEvmDeposits(testContext: TestContext) { + + const depositTests = Promise.all([ testSuccessiveDepositEvm('Eth', 'Dot', testContext), testSuccessiveDepositEvm('Flip', 'Btc', testContext), @@ -204,12 +206,12 @@ export async function testEvmDeposits(testContext: TestContext) { testNoDuplicateWitnessing('ArbEth', 'Usdc', testContext), ]); - const multipleTxSwapsTest = Promise.all([ - testTxMultipleVaultSwaps('Eth', 'Dot', testContext), - testTxMultipleVaultSwaps('Eth', 'Flip', testContext), - testTxMultipleVaultSwaps('ArbEth', 'Dot', testContext), - testTxMultipleVaultSwaps('ArbEth', 'Flip', testContext), - ]); + // const multipleTxSwapsTest = Promise.all([ + // testTxMultipleVaultSwaps('Eth', 'Dot', testContext), + // testTxMultipleVaultSwaps('Eth', 'Flip', testContext), + // testTxMultipleVaultSwaps('ArbEth', 'Dot', testContext), + // testTxMultipleVaultSwaps('ArbEth', 'Flip', testContext), + // ]); const doubleDepositTests = Promise.all([ testDoubleDeposit('Eth', 'Dot', testContext), @@ -221,7 +223,7 @@ export async function testEvmDeposits(testContext: TestContext) { await Promise.all([ depositTests, noDuplicatedWitnessingTest, - multipleTxSwapsTest, + // multipleTxSwapsTest, doubleDepositTests, ]); } diff --git a/bouncer/tests/fast_bouncer.test.ts b/bouncer/tests/fast_bouncer.test.ts index afcd80011b..72f45816b6 100644 --- a/bouncer/tests/fast_bouncer.test.ts +++ b/bouncer/tests/fast_bouncer.test.ts @@ -1,6 +1,6 @@ import { describe } from 'vitest'; import { testBoostingSwap } from './boost'; -import { testVaultSwapFeeCollection } from './vault_swap_fee_collection'; +import { testVaultSwap } from './vault_swap_tests'; import { testPolkadotRuntimeUpdate } from './polkadot_runtime_update'; import { checkSolEventAccountsClosure } from '../shared/sol_vault_swap'; import { checkAvailabilityAllSolanaNonces } from '../shared/utils'; @@ -15,7 +15,6 @@ import { testDCASwaps } from './DCA_test'; import { testCancelOrdersBatch } from './create_and_delete_multiple_orders'; import { depositChannelCreation } from './request_swap_deposit_address_with_affiliates'; import { testBrokerLevelScreening } from './broker_level_screening'; -import { legacyEvmVaultSwaps } from './legacy_vault_swap'; import { testFundRedeem } from './fund_redeem'; import { concurrentTest, serialTest } from '../shared/utils/vitest'; @@ -29,7 +28,7 @@ describe('ConcurrentTests', () => { concurrentTest('SwapLessThanED', swapLessThanED, 300); concurrentTest('AllSwaps', testAllSwaps, numberOfNodes === 1 ? 1200 : 1800); // TODO: find out what the 3-node timeout should be - concurrentTest('EvmDeposits', testEvmDeposits, 250); + concurrentTest('EvmDeposits', testEvmDeposits, 350); concurrentTest('FundRedeem', testFundRedeem, 1000); concurrentTest('MultipleMembersGovernance', testMultipleMembersGovernance, 120); concurrentTest('LpApi', testLpApi, 200); @@ -40,8 +39,7 @@ describe('ConcurrentTests', () => { concurrentTest('CancelOrdersBatch', testCancelOrdersBatch, 240); concurrentTest('DepositChannelCreation', depositChannelCreation, 360); concurrentTest('BrokerLevelScreening', testBrokerLevelScreening, 300); - concurrentTest('legacyEvmVaultSwaps', legacyEvmVaultSwaps, 300); - concurrentTest('VaultSwapFeeCollection', testVaultSwapFeeCollection, 600); + concurrentTest('VaultSwap', testVaultSwap, 600); // Tests that only work if there is more than one node if (numberOfNodes > 1) { diff --git a/bouncer/tests/legacy_vault_swap.ts b/bouncer/tests/legacy_vault_swap.ts deleted file mode 100644 index ec06537bd2..0000000000 --- a/bouncer/tests/legacy_vault_swap.ts +++ /dev/null @@ -1,162 +0,0 @@ -import { InternalAsset as Asset, InternalAssets as Assets } from '@chainflip/cli'; -import { randomBytes } from 'crypto'; -import Web3 from 'web3'; -import assert from 'assert'; -import { - amountToFineAmount, - assetContractId, - assetDecimals, - ccmSupportedChains, - chainContractId, - chainFromAsset, - chainGasAsset, - createEvmWalletAndFund, - defaultAssetAmounts, - getContractAddress, - getEvmEndpoint, - newAddress, - observeBalanceIncrease, - observeSwapRequested, - SwapRequestType, - TransactionOrigin, -} from '../shared/utils'; -import { observeEvent } from '../shared/utils/substrate'; -import { getBalance } from '../shared/get_balance'; -import { newVaultSwapCcmMetadata } from '../shared/swapping'; -import { getEvmVaultAbi } from '../shared/contract_interfaces'; -import { approveEvmTokenVault } from '../shared/evm_vault_swap'; -import { TestContext } from '../shared/utils/test_context'; -import { Logger } from '../shared/utils/logger'; - -async function legacyEvmVaultSwap( - logger: Logger, - sourceAsset: Asset, - destAsset: Asset, - ccmSwap: boolean = false, -) { - const destAddress = ccmSwap - ? getContractAddress(chainFromAsset(destAsset), 'CFTESTER') - : await newAddress(destAsset, randomBytes(32).toString('hex')); - const destBalanceBefore = await getBalance(destAsset, destAddress); - const sourceChain = chainFromAsset(sourceAsset); - const destChain = chainFromAsset(destAsset); - const amount = amountToFineAmount(defaultAssetAmounts(sourceAsset), assetDecimals(sourceAsset)); - - // Only EVM have legacy vault swaps - assert(ccmSupportedChains.includes(sourceChain)); - - const web3 = new Web3(getEvmEndpoint(sourceChain)); - const vaultAddress = getContractAddress(sourceChain, 'VAULT'); - const vaultContract = new web3.eth.Contract( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (await getEvmVaultAbi()) as any, - vaultAddress, - ); - const evmWallet = await createEvmWalletAndFund(sourceAsset); - - const cfParametersList = ['', '0x', 'deadbeef', '0xdeadbeef', 'deadc0de', '0xdeadc0de']; - const cfParameters = Math.floor(Math.random() * cfParametersList.length); - - if (chainGasAsset(sourceChain) !== sourceAsset) { - // Doing effectively infinite approvals to make sure it doesn't fail. - await approveEvmTokenVault(sourceAsset, (BigInt(amount) * 100n).toString(), evmWallet); - } - - let data; - if (!ccmSwap) { - if (chainGasAsset(sourceChain) === sourceAsset) { - data = vaultContract.methods - .xSwapNative( - chainContractId(destChain), - destAddress, - assetContractId(destAsset), - cfParameters, - ) - .encodeABI(); - } else { - data = vaultContract.methods - .xSwapToken( - chainContractId(destChain), - destAddress, - assetContractId(destAsset), - getContractAddress(sourceChain, sourceAsset), - Number(amount), - cfParameters, - ) - .encodeABI(); - } - } else { - const ccmSwapMetadata = await newVaultSwapCcmMetadata(sourceAsset, destAsset); - if (chainGasAsset(sourceChain) === sourceAsset) { - data = vaultContract.methods - .xCallNative( - chainContractId(destChain), - destAddress, - assetContractId(destAsset), - ccmSwapMetadata.message, - ccmSwapMetadata.gasBudget, - cfParameters, - ) - .encodeABI(); - } else { - data = vaultContract.methods - .xCallToken( - chainContractId(destChain), - destAddress, - assetContractId(destAsset), - ccmSwapMetadata.message, - ccmSwapMetadata.gasBudget, - getContractAddress(sourceChain, sourceAsset), - amount, - cfParameters, - ) - .encodeABI(); - } - } - - const tx = { - to: vaultAddress, - data, - gas: sourceChain === 'Arbitrum' ? 32000000 : 5000000, - value: chainGasAsset(sourceChain) === sourceAsset ? amount : '0', - }; - - const signedTx = await web3.eth.accounts.signTransaction(tx, evmWallet.privateKey); - const receipt = await web3.eth.sendSignedTransaction(signedTx.rawTransaction as string); - - logger.debug(`Vault swap executed, tx hash: ${receipt.transactionHash}`); - - // Look after Swap Requested of data.origin.Vault.tx_hash - const swapRequestedHandle = observeSwapRequested( - sourceAsset, - destAsset, - { type: TransactionOrigin.VaultSwapEvm, txHash: receipt.transactionHash }, - SwapRequestType.Regular, - ); - - const swapRequestId = Number((await swapRequestedHandle).data.swapRequestId.replaceAll(',', '')); - logger.debug(`${sourceAsset} swap via vault, swapRequestId: ${swapRequestId}`); - - // Wait for the swap to complete - await observeEvent(`swapping:SwapRequestCompleted`, { - test: (event) => Number(event.data.swapRequestId.replaceAll(',', '')) === swapRequestId, - }).event; - - await observeEvent(`swapping:SwapExecuted`, { - test: (event) => Number(event.data.swapRequestId.replaceAll(',', '')) === swapRequestId, - historicalCheckBlocks: 10, - }).event; - - logger.debug(`swapRequestId: ${swapRequestId} executed. Waiting for balance to increase.`); - await observeBalanceIncrease(destAsset, destAddress, destBalanceBefore); - logger.debug(`swapRequestId: ${swapRequestId} - swap success`); -} - -export async function legacyEvmVaultSwaps(testContext: TestContext) { - await Promise.all([ - legacyEvmVaultSwap(testContext.logger, Assets.Eth, Assets.ArbUsdc), - legacyEvmVaultSwap(testContext.logger, Assets.ArbEth, Assets.Eth, true), - legacyEvmVaultSwap(testContext.logger, Assets.ArbUsdc, Assets.Usdt), - legacyEvmVaultSwap(testContext.logger, Assets.Usdc, Assets.Eth, true), - ]); -} diff --git a/bouncer/tests/vault_swap_fee_collection.ts b/bouncer/tests/vault_swap_tests.ts similarity index 79% rename from bouncer/tests/vault_swap_fee_collection.ts rename to bouncer/tests/vault_swap_tests.ts index ab62e1303d..b639a70223 100644 --- a/bouncer/tests/vault_swap_fee_collection.ts +++ b/bouncer/tests/vault_swap_tests.ts @@ -12,7 +12,7 @@ import { import { getEarnedBrokerFees } from './broker_fee_collection'; import { openPrivateBtcChannel, registerAffiliate } from '../shared/btc_vault_swap'; import { setupBrokerAccount } from '../shared/setup_account'; -import { performVaultSwap } from '../shared/perform_swap'; +import { executeVaultSwap, performVaultSwap } from '../shared/perform_swap'; import { prepareSwap } from '../shared/swapping'; import { getChainflipApi } from '../shared/utils/substrate'; import { getBalance } from '../shared/get_balance'; @@ -22,6 +22,51 @@ import { Logger } from '../shared/utils/logger'; // Fee to use for the broker and affiliates const commissionBps = 100; +async function testRefundVaultSwap(logger: Logger) { + logger.info('Starting refund vault swap test...'); + + const inputAsset = Assets.Btc; + const destAsset = Assets.Usdc; + const balanceObserveTimeout = 60; + const depositAmount = defaultAssetAmounts(inputAsset); + const destAddress = await newAddress('Usdc', 'BTC_VAULT_SWAP_REFUND' + Math.random() * 100); + const refundAddress = await newAddress('Btc', 'BTC_VAULT_SWAP_REFUND' + Math.random() * 100); + const foKParams = { + retryDurationBlocks: 100, + refundAddress, + minPriceX128: '0', + }; + + logger.info('Sending vault swap...'); + + await executeVaultSwap( + inputAsset, + destAsset, + destAddress, + undefined, + depositAmount, + 0, // boostFeeBps + foKParams, + ); + + logger.info('Waiting for refund...'); + + let btcBalance = false; + + for (let i = 0; i < balanceObserveTimeout; i++) { + const refundAddressBalance = await getBalance(Assets.Btc, refundAddress); + if (refundAddressBalance !== '0') { + btcBalance = true; + break; + } + await sleep(1000); + } + + assert(btcBalance, `Vault swap refund failed 🙅‍♂️.`); + + logger.info('Refund vault swap completed ✅.'); +} + async function testWithdrawCollectedAffiliateFees( broker: KeyringPair, affiliateAccountId: string, @@ -134,7 +179,7 @@ async function testFeeCollection( return Promise.resolve([broker, affiliateId, refundAddress]); } -export async function testVaultSwapFeeCollection(testContext: TestContext) { +export async function testVaultSwap(testContext: TestContext) { await Promise.all([ testFeeCollection(Assets.Eth, testContext), testFeeCollection(Assets.ArbEth, testContext), @@ -144,4 +189,5 @@ export async function testVaultSwapFeeCollection(testContext: TestContext) { // Test the affiliate withdrawal functionality const [broker, affiliateId, refundAddress] = await testFeeCollection(Assets.Btc, testContext); await testWithdrawCollectedAffiliateFees(broker, affiliateId, refundAddress, testContext.logger); + await testRefundVaultSwap(testContext.logger); } diff --git a/state-chain/cf-integration-tests/src/solana.rs b/state-chain/cf-integration-tests/src/solana.rs index 9f4a05d3e7..c641fe2a21 100644 --- a/state-chain/cf-integration-tests/src/solana.rs +++ b/state-chain/cf-integration-tests/src/solana.rs @@ -18,7 +18,7 @@ use cf_chains::{ ExecutexSwapAndCallError, ForeignChainAddress, RequiresSignatureRefresh, SetAggKeyWithAggKey, SetAggKeyWithAggKeyError, Solana, SwapOrigin, TransactionBuilder, }; -use cf_primitives::{AccountRole, AuthorityCount, ForeignChain, SwapRequestId}; +use cf_primitives::{AccountRole, AuthorityCount, Beneficiary, ForeignChain, SwapRequestId}; use cf_test_utilities::{assert_events_match, assert_has_matching_event}; use cf_utilities::bs58_array; use codec::Encode; @@ -378,7 +378,7 @@ fn vault_swap_deposit_witness( deposit_metadata, tx_id: Default::default(), deposit_details: (), - broker_fee: None, + broker_fee: Some(Beneficiary { account: BROKER.into(), bps: 0 }), affiliate_fees: Default::default(), refund_params: Some(REFUND_PARAMS), dca_params: None, diff --git a/state-chain/cf-integration-tests/src/swapping.rs b/state-chain/cf-integration-tests/src/swapping.rs index 3156835a7c..0fbed10b44 100644 --- a/state-chain/cf-integration-tests/src/swapping.rs +++ b/state-chain/cf-integration-tests/src/swapping.rs @@ -7,7 +7,7 @@ use crate::{ fund_authorities_and_join_auction, new_account, register_refund_addresses, setup_account_and_peer_mapping, Cli, Network, }, - witness_call, witness_ethereum_rotation_broadcast, witness_rotation_broadcasts, + witness_call, witness_ethereum_rotation_broadcast, witness_rotation_broadcasts, BROKER, }; use cf_amm::{ math::{price_at_tick, Price, Tick}, @@ -23,8 +23,8 @@ use cf_chains::{ RetryPolicy, SwapOrigin, TransactionBuilder, TransferAssetParams, }; use cf_primitives::{ - AccountId, AccountRole, Asset, AssetAmount, AuthorityCount, SwapId, FLIPPERINOS_PER_FLIP, - GENESIS_EPOCH, STABLE_ASSET, SWAP_DELAY_BLOCKS, + AccountId, AccountRole, Asset, AssetAmount, AuthorityCount, Beneficiary, SwapId, + FLIPPERINOS_PER_FLIP, GENESIS_EPOCH, STABLE_ASSET, SWAP_DELAY_BLOCKS, }; use cf_test_utilities::{assert_events_eq, assert_events_match, assert_has_matching_event}; use cf_traits::{AdjustedFeeEstimationApi, AssetConverter, BalanceApi, EpochInfo, SwapType}; @@ -530,7 +530,7 @@ fn vault_swap_deposit_witness( deposit_metadata: Some(ccm_deposit_metadata_mock()), tx_id: Default::default(), deposit_details: DepositDetails { tx_hashes: None }, - broker_fee: None, + broker_fee: Some(Beneficiary { account: BROKER.into(), bps: 0 }), affiliate_fees: Default::default(), refund_params: Some(ETH_REFUND_PARAMS), dca_params: None, diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 16c08fb982..550a929148 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -32,7 +32,7 @@ use cf_chains::{ ChannelRefundParametersDecoded, ConsolidateCall, DepositChannel, DepositDetailsToTransactionInId, DepositOriginType, ExecutexSwapAndCall, FetchAssetParams, ForeignChainAddress, IntoTransactionInIdForAnyChain, RefundParametersExtended, RejectCall, - SwapOrigin, TransferAssetParams, + SwapOrigin, TransferAssetParams, TransferFallback, }; use cf_primitives::{ AccountRole, AffiliateShortId, Affiliates, Asset, AssetAmount, BasisPoints, Beneficiaries, @@ -920,6 +920,9 @@ pub mod pallet { NetworkFeeDeductionFromBoostSet { deduction_percent: Percent, }, + VaultSwapRefunded { + tx_id: TransactionInIdFor, + }, } #[derive(CloneNoBound, PartialEqNoBound, EqNoBound)] @@ -2015,6 +2018,17 @@ impl, I: 'static> Pallet { Ok(()) } + /// Returns true if the vault swap should be rejected. + /// + /// A vault swap should be rejected if the broker fee is not set or the account is not a broker. + fn should_reject_vault_swap(broker_fee: &Option>) -> bool { + match broker_fee { + Some(Beneficiary { account, .. }) => + !T::AccountRoleRegistry::has_account_role(account, AccountRole::Broker), + _ => true, + } + } + fn assemble_broker_fees( broker_fee: Option>, affiliate_fees: Affiliates, @@ -2189,6 +2203,10 @@ impl, I: 'static> Pallet { boost_fee, }: VaultDepositWitness, ) { + if Self::should_reject_vault_swap(&broker_fee) { + return; + } + let destination_address_internal = match T::AddressConverter::decode_and_validate_address_for_asset( destination_address.clone(), @@ -2466,6 +2484,28 @@ impl, I: 'static> Pallet { boost_fee, } = vault_deposit_witness.clone(); + if Self::should_reject_vault_swap(&broker_fee) { + // TODO: We are going to make refund_params mandatory so we can remove this if let check + // as soon the other PR is merged. + if let Some(refund_params) = refund_params { + if let Ok(api_call) = + >::new_unsigned( + TransferAssetParams { + asset: source_asset, + amount: deposit_amount, + to: refund_params.refund_address, + }, + ) { + T::Broadcaster::threshold_sign_and_broadcast(api_call); + Self::deposit_event(Event::VaultSwapRefunded { tx_id: tx_id.clone() }); + } else { + log_or_panic!("Failed to create refund api call for vault swap."); + } + }; + + return; + } + let boost_status = BoostedVaultTransactions::::get(&tx_id).unwrap_or(BoostStatus::NotBoosted); diff --git a/state-chain/pallets/cf-ingress-egress/src/tests.rs b/state-chain/pallets/cf-ingress-egress/src/tests.rs index 94175a5e4d..983e2ab338 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests.rs @@ -48,7 +48,7 @@ use frame_support::{ traits::{Hooks, OriginTrait}, weights::Weight, }; -use sp_core::{bounded_vec, H160}; +use sp_core::{bounded_vec, H160, U256}; use sp_runtime::{DispatchError, DispatchResult, Percent}; const ALICE_ETH_ADDRESS: EthereumAddress = H160([100u8; 20]); @@ -2045,30 +2045,11 @@ fn charge_no_broker_fees_on_unknown_primary_broker() { 0 )); - // The request is recorded as not having any broker fees: - assert_eq!( - MockSwapRequestHandler::::get_swap_requests(), - vec![MockSwapRequest { - input_asset: INPUT_ASSET, - output_asset: OUTPUT_ASSET, - input_amount: INPUT_AMOUNT, - swap_type: SwapRequestType::Regular { - output_action: SwapOutputAction::Egress { - output_address, - ccm_deposit_metadata: None - } - }, - broker_fees: Default::default(), - origin: SwapOrigin::Vault { - tx_id: cf_chains::TransactionInIdForAnyChain::Evm(H256::default()), - broker_id: Some(NOT_A_BROKER), - }, - },] - ); - - assert_has_event::(RuntimeEvent::IngressEgress(PalletEvent::UnknownBroker { - broker_id: NOT_A_BROKER, + assert_has_event::(RuntimeEvent::IngressEgress(PalletEvent::VaultSwapRefunded { + tx_id: H256::default(), })); + + assert!(MockSwapRequestHandler::::get_swap_requests().is_empty()); }); } @@ -2208,7 +2189,7 @@ fn failed_ccm_deposit_can_deposit_event() { Some(ccm_deposit_metadata.clone()), Default::default(), DepositDetails { tx_hashes: None }, - Beneficiary { account: 0, bps: 0 }, + Beneficiary { account: BROKER, bps: 0 }, Default::default(), ETH_REFUND_PARAMS, None, @@ -2358,3 +2339,63 @@ fn ignore_change_of_minimum_deposit_if_deposit_is_not_boosted() { .is_ok()); }); } + +#[test] +fn gets_refunded_if_vault_transaction_was_aborted() { + new_test_ext().execute_with(|| { + let tx_id = H256::default(); + + let vault_swap = VaultDepositWitness { + input_asset: Asset::Eth.try_into().unwrap(), + deposit_address: Default::default(), + channel_id: Some(0), + deposit_amount: 100, + deposit_details: Default::default(), + output_asset: Asset::Eth, + destination_address: EncodedAddress::Eth(Default::default()), + deposit_metadata: Default::default(), + tx_id, + broker_fee: None, + affiliate_fees: Default::default(), + refund_params: Some(ChannelRefundParameters { + retry_duration: 0, + min_price: U256::from(0), + refund_address: H160::default(), + }), + dca_params: None, + boost_fee: 0, + }; + + IngressEgress::process_vault_swap_request_prewitness(0, vault_swap.clone()); + + IngressEgress::process_vault_swap_request_full_witness(0, vault_swap); + + assert_has_matching_event!( + Test, + RuntimeEvent::IngressEgress(Event::VaultSwapRefunded { tx_id: _ }) + ); + + assert!( + MockSwapRequestHandler::::get_swap_requests().is_empty(), + "No swaps should have been triggered!" + ); + + assert!( + MockEgressBroadcaster::get_pending_api_calls().len() == 1, + "Refund broadcast should have been scheduled!" + ); + }); +} + +#[test] +fn reject_requirements_test() { + new_test_ext().execute_with(|| { + let should_fail = Some(Beneficiary { account: ALICE, bps: 0 }); + let should_succeed = Some(Beneficiary { account: BROKER, bps: 0 }); + + assert!(IngressEgress::should_reject_vault_swap(&should_fail)); + assert!(IngressEgress::should_reject_vault_swap(&None)); + + assert!(!IngressEgress::should_reject_vault_swap(&should_succeed)); + }); +}