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: EVM Vault swaps support for Broker and affiliates #5408

Merged
merged 23 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
228cbef
feat: add cf-parameters versioning
albert-llimos Nov 7, 2024
6add0fc
chore: update with SDK changes
albert-llimos Nov 7, 2024
ab416fe
chore: update broker and affiliates in engine and bouncer
albert-llimos Nov 8, 2024
269745f
chore: revert all_swaps
albert-llimos Nov 8, 2024
f6a2ec8
chore: update to 1.8.0-rc.3
albert-llimos Nov 8, 2024
693b992
chore: update broker and affiliates in engine and bouncer
albert-llimos Nov 8, 2024
8d07d54
chore: revert all_swaps
albert-llimos Nov 8, 2024
b6b62ae
chore: update to 1.8.0-rc.3
albert-llimos Nov 8, 2024
c607c8e
Merge branch 'feat/cf-parameters-broker-affiliates' of github.com:cha…
albert-llimos Nov 8, 2024
fab6ad1
chore: fix vault_swap_request
albert-llimos Nov 8, 2024
01455b6
chore: make brokerFees optional
albert-llimos Nov 11, 2024
ba6fe7b
chore: add encoding test and update evm test
albert-llimos Nov 11, 2024
7a3b94a
chore: merge from main
albert-llimos Nov 11, 2024
1e6a3da
chore: use AffiliateAndFee instead of ShortId
albert-llimos Nov 11, 2024
5ebde8a
chore: improve tests
albert-llimos Nov 11, 2024
3dde9e2
chore: merge from main
albert-llimos Nov 13, 2024
44736e6
chore: refactor and plug in broker and affiliate fees
albert-llimos Nov 13, 2024
22967a2
chore: fix bouncer lint
albert-llimos Nov 13, 2024
575915f
feat: make boost_fee not optional
albert-llimos Nov 13, 2024
2930e68
chore: fix bug in sdk
albert-llimos Nov 13, 2024
d807601
chore: address comments
albert-llimos Nov 14, 2024
53d82dd
feat: make refund_params non-optional for vault swaps in extrinsic
albert-llimos Nov 14, 2024
2d44dfb
chore: fix merge conflicts
albert-llimos Nov 14, 2024
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
2 changes: 1 addition & 1 deletion bouncer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"prettier:write": "prettier --write ."
},
"dependencies": {
"@chainflip/cli": "1.8.0-rc.2",
"@chainflip/cli": "1.8.0-rc.5",
"@chainflip/utils": "^0.4.0",
"@coral-xyz/anchor": "^0.30.1",
"@iarna/toml": "^2.2.5",
Expand Down
25 changes: 10 additions & 15 deletions bouncer/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 12 additions & 1 deletion bouncer/shared/evm_vault_swap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { HDNodeWallet } from 'ethers';
import { randomBytes } from 'crypto';
import { PublicKey, sendAndConfirmTransaction, Keypair } from '@solana/web3.js';
import { getAssociatedTokenAddressSync, TOKEN_PROGRAM_ID } from '@solana/spl-token';
import Keyring from '../polkadot/keyring';
import {
observeBalanceIncrease,
getContractAddress,
Expand Down Expand Up @@ -54,6 +55,10 @@ export async function executeVaultSwap(
fillOrKillParams?: FillOrKillParamsX128,
dcaParams?: DcaParams,
wallet?: HDNodeWallet,
brokerFees?: {
account: string;
commissionBps: number;
},
): ReturnType<typeof executeSwap> {
const srcChain = chainFromAsset(sourceAsset);
const destChain = chainFromAsset(destAsset);
Expand All @@ -68,6 +73,11 @@ export async function executeVaultSwap(

const evmWallet = wallet ?? (await createEvmWalletAndFund(sourceAsset));

const brokerComission = brokerFees ?? {
account: new Keyring({ type: 'sr25519' }).createFromUri('//BROKER_1').address,
commissionBps: 1,
};

if (erc20Assets.includes(sourceAsset)) {
// Doing effectively infinite approvals to make sure it doesn't fail.
// eslint-disable-next-line @typescript-eslint/no-use-before-define
Expand Down Expand Up @@ -104,12 +114,13 @@ export async function executeVaultSwap(
message: messageMetadata.message,
ccmAdditionalData: messageMetadata.ccmAdditionalData,
},
brokerFees: brokerComission,
// The SDK will encode these parameters and the ccmAdditionalData
// into the `cfParameters` field for the vault swap.
boostFeeBps,
fillOrKillParams: fokParams,
dcaParams,
beneficiaries: undefined,
affiliateFees: undefined,
} as ExecuteSwapParams,
networkOptions,
txOptions,
Expand Down
4 changes: 2 additions & 2 deletions bouncer/tests/evm_deposits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ async function testTxMultipleVaultSwaps(sourceAsset: Asset, destAsset: Asset) {
assetContractId(destAsset),
getContractAddress(chainFromAsset(sourceAsset), sourceAsset),
amount,
// Encoded EVM refund address and no other swap parameters.
'0x0000000000000e879c89cad7076b347bde13c99cf2c33e7299b60000000000000000000000000000000000000000000000000000000000000000000000',
// Dummy encoded data containing a refund address and a broker accountId.
'0x00000100000000020202020202020202020202020202020202020200000000000000000000000000000000000000000000000000000000000000000000000303030303030303030303030303030303030303030303030303030303030303040000',
numSwaps,
)
.encodeABI();
Expand Down
52 changes: 24 additions & 28 deletions engine/src/witness/arb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@ mod chain_tracking;

use std::{collections::HashMap, sync::Arc};

use cf_chains::{assets::arb::Asset as ArbAsset, evm::DepositDetails, Arbitrum};
use cf_primitives::{AffiliateShortId, EpochIndex};
use cf_chains::{address::EncodedAddress, evm::DepositDetails, Arbitrum, CcmDepositMetadata};
use cf_primitives::{
chains::assets::arb::Asset as ArbAsset, Asset, AssetAmount, Beneficiary, EpochIndex,
TransactionHash,
};
use cf_utilities::task_scope::Scope;
use futures_core::Future;
use itertools::Itertools;
use sp_core::H160;

use crate::{
Expand All @@ -18,7 +22,7 @@ use crate::{
stream_api::{StreamApi, FINALIZED},
STATE_CHAIN_CONNECTION,
},
witness::evm::erc20_deposits::usdc::UsdcEvents,
witness::{common::cf_parameters::VaultSwapParameters, evm::erc20_deposits::usdc::UsdcEvents},
};

use super::{
Expand Down Expand Up @@ -81,11 +85,10 @@ where
.get(&ArbAsset::ArbUsdc)
.context("ArbitrumSupportedAssets does not include USDC")?;

let supported_arb_erc20_assets: HashMap<H160, cf_primitives::Asset> =
supported_arb_erc20_assets
.into_iter()
.map(|(asset, address)| (address, asset.into()))
.collect();
let supported_arb_erc20_assets: HashMap<H160, Asset> = supported_arb_erc20_assets
.into_iter()
.map(|(asset, address)| (address, asset.into()))
.collect();

let arb_source = EvmSource::<_, Arbitrum>::new(arb_client.clone())
.strictly_monotonic()
Expand Down Expand Up @@ -162,7 +165,7 @@ where
process_call,
arb_client.clone(),
vault_address,
cf_primitives::Asset::ArbEth,
Asset::ArbEth,
cf_primitives::ForeignChain::Arbitrum,
supported_arb_erc20_assets,
)
Expand All @@ -175,11 +178,6 @@ where

struct ArbCallBuilder {}

use cf_chains::{address::EncodedAddress, CcmDepositMetadata, ChannelRefundParameters};
use cf_primitives::{
Asset, AssetAmount, BasisPoints, Beneficiaries, DcaParameters, TransactionHash,
};

impl super::evm::vault::IngressCallBuilder for ArbCallBuilder {
type Chain = Arbitrum;

Expand All @@ -190,11 +188,7 @@ impl super::evm::vault::IngressCallBuilder for ArbCallBuilder {
destination_address: EncodedAddress,
deposit_metadata: Option<CcmDepositMetadata>,
tx_hash: TransactionHash,
_broker_fees: Beneficiaries<AffiliateShortId>,
refund_params: Option<ChannelRefundParameters>,
dca_params: Option<DcaParameters>,
// This is only to be checked in the pre-witnessed version
boost_fee: Option<BasisPoints>,
vault_swap_parameters: VaultSwapParameters,
) -> state_chain_runtime::RuntimeCall {
state_chain_runtime::RuntimeCall::ArbitrumIngressEgress(
pallet_cf_ingress_egress::Call::vault_swap_request {
Expand All @@ -205,15 +199,17 @@ impl super::evm::vault::IngressCallBuilder for ArbCallBuilder {
deposit_metadata,
tx_hash,
deposit_details: Box::new(DepositDetails { tx_hashes: Some(vec![tx_hash.into()]) }),
// Defaulting to no broker fees until PRO-1743 is completed.
broker_fee: cf_primitives::Beneficiary {
account: sp_runtime::AccountId32::new([0; 32]),
bps: 0,
},
affiliate_fees: Default::default(),
boost_fee: boost_fee.unwrap_or_default(),
dca_params,
refund_params: refund_params.map(Box::new),
broker_fee: vault_swap_parameters.broker_fee,
affiliate_fees: vault_swap_parameters
.affiliate_fees
.into_iter()
.map(|entry| Beneficiary { account: entry.affiliate.into(), bps: entry.fee.into() })
.collect_vec()
.try_into()
.expect("runtime supports at least as many affiliates as we allow in cf_parameters encoding"),
boost_fee: vault_swap_parameters.boost_fee,
dca_params: vault_swap_parameters.dca_params,
refund_params: Some(Box::new(vault_swap_parameters.refund_params)),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come refund params are optional on the extrinsic but we never use None?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's because we wanted to have some flexibility as we've been going back and forth between making this mandatory or not. I think we could consider removing the option since we are now enforcing it in Vault swaps. Wdyt? @dandanlen @msgmaxim

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's remove Option

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

},
)
}
Expand Down
2 changes: 1 addition & 1 deletion engine/src/witness/btc/vault_swaps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ mod tests {
chunk_interval: 2,
boost_fee: 5,
broker_fee: 10,
affiliates: bounded_vec![AffiliateAndFee { affiliate: 17, fee: 7 }],
affiliates: bounded_vec![cf_primitives::AffiliateAndFee { affiliate: 17, fee: 7 }],
},
});

Expand Down
57 changes: 53 additions & 4 deletions engine/src/witness/common/cf_parameters.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
use cf_chains::{CcmAdditionalData, ChannelRefundParameters};
use cf_primitives::{AffiliateShortId, BasisPoints, Beneficiaries, DcaParameters};
use cf_primitives::{
AccountId, AffiliateAndFee, BasisPoints, Beneficiary, DcaParameters, MAX_AFFILIATES,
};
use codec::{Decode, Encode, MaxEncodedLen};
use scale_info::TypeInfo;
use sp_core::ConstU32;
use sp_runtime::BoundedVec;

#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, Clone, PartialEq, Debug)]
pub enum VersionedCfParameters<CcmData = ()> {
Expand All @@ -21,19 +25,27 @@ pub type VersionedCcmCfParameters = VersionedCfParameters<CcmAdditionalData>;
pub struct VaultSwapParameters {
pub refund_params: ChannelRefundParameters,
pub dca_params: Option<DcaParameters>,
pub boost_fee: Option<BasisPoints>,
pub broker_fees: Beneficiaries<AffiliateShortId>,
pub boost_fee: BasisPoints,
pub broker_fee: Beneficiary<AccountId>,
pub affiliate_fees: BoundedVec<AffiliateAndFee, ConstU32<MAX_AFFILIATES>>,
}

#[cfg(test)]
mod tests {
use super::*;
use cf_chains::MAX_CCM_ADDITIONAL_DATA_LENGTH;
use cf_chains::{ChannelRefundParameters, ForeignChainAddress, MAX_CCM_ADDITIONAL_DATA_LENGTH};

const MAX_VAULT_SWAP_PARAMETERS_LENGTH: u32 = 1_000;
const MAX_CF_PARAM_LENGTH: u32 =
MAX_CCM_ADDITIONAL_DATA_LENGTH + MAX_VAULT_SWAP_PARAMETERS_LENGTH;

const REFERENCE_EXPECTED_ENCODED: &[u8] = &[
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe subjective, but I kind of like the way I broke down individual fields in check_utxo_encoding (https://github.com/chainflip-io/chainflip-backend/blob/main/state-chain/chains/src/btc/vault_swap_encoding.rs#L144) so we could see which bytes correspond to which field. Probably more important for BTC though where we were trying to compress the encoding.

0, 1, 0, 0, 0, 0, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
3, 3, 3, 4, 0, 0,
];

#[test]
fn test_cf_parameters_max_length() {
assert!(
Expand All @@ -44,4 +56,41 @@ mod tests {
MAX_VAULT_SWAP_PARAMETERS_LENGTH as usize >= VaultSwapParameters::max_encoded_len()
);
}

#[test]
fn test_versioned_cf_parameters() {
let vault_swap_parameters = VaultSwapParameters {
refund_params: ChannelRefundParameters {
retry_duration: 1,
refund_address: ForeignChainAddress::Eth(sp_core::H160::from([2; 20])),
min_price: Default::default(),
},
dca_params: None,
boost_fee: 0,
broker_fee: Beneficiary { account: AccountId::new([3; 32]), bps: 4 },
affiliate_fees: sp_core::bounded_vec![],
};

let cf_parameters = CfParameters::<()> {
ccm_additional_data: (),
vault_swap_parameters: vault_swap_parameters.clone(),
};

let mut encoded = VersionedCfParameters::V0(cf_parameters).encode();

assert_eq!(encoded, REFERENCE_EXPECTED_ENCODED);

let ccm_cf_parameters = CfParameters {
ccm_additional_data: CcmAdditionalData::default(),
vault_swap_parameters,
};

encoded = VersionedCcmCfParameters::V0(ccm_cf_parameters).encode();

// Extra byte for the empty ccm metadata
let mut expected_encoded = vec![0];
expected_encoded.extend_from_slice(REFERENCE_EXPECTED_ENCODED);
Copy link
Contributor

@msgmaxim msgmaxim Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I believe you can just convert to Vec with Vec::from (or alternatively compare slices with &encoded[..] or something like that)


assert_eq!(encoded, expected_encoded);
}
}
Loading
Loading