diff --git a/api/lib/src/lib.rs b/api/lib/src/lib.rs index ee857ebf7e..93d0d64afb 100644 --- a/api/lib/src/lib.rs +++ b/api/lib/src/lib.rs @@ -521,7 +521,7 @@ pub trait BrokerApi: SignedExtrinsicApi + StorageApi + Sized + Send + Sync + 'st affiliate_id: AccountId32, short_id: Option, ) -> Result { - let next_id = if let Some(short_id) = short_id { + let register_as_id = if let Some(short_id) = short_id { short_id } else { let affiliates = @@ -534,25 +534,17 @@ pub trait BrokerApi: SignedExtrinsicApi + StorageApi + Sized + Send + Sync + 'st return Ok(*existing_short_id); } - // Find the lowest unused short id - let mut used_ids: Vec = + // Auto assign the lowest unused short id + let used_ids: Vec = affiliates.into_iter().map(|(short_id, _)| short_id).collect(); - used_ids.sort_unstable(); - let lowest_unused = move || { - for (index, assigned_id) in (0..=u8::MAX).zip(used_ids) { - if AffiliateShortId::from(index) != assigned_id { - return Ok(index.into()); - } - } - Err(anyhow!("No unused affiliate short IDs available")) - }; - lowest_unused()? + find_lowest_unused_short_id(&used_ids)? }; - let (_, events, ..) = - self.submit_signed_extrinsic_with_dry_run( - pallet_cf_swapping::Call::register_affiliate { affiliate_id, short_id: next_id }, - ) + let (_, events, ..) = self + .submit_signed_extrinsic_with_dry_run(pallet_cf_swapping::Call::register_affiliate { + affiliate_id, + short_id: register_as_id, + }) .await? .until_in_block() .await?; @@ -705,6 +697,26 @@ pub fn generate_ethereum_key( )) } +fn find_lowest_unused_short_id(used_ids: &[AffiliateShortId]) -> Result { + let used_id_len = used_ids.len(); + if used_ids.is_empty() { + Ok(AffiliateShortId::from(0)) + } else if used_id_len > u8::MAX as usize { + bail!("No unused affiliate short IDs available") + } else { + let mut used_ids = used_ids.to_vec(); + used_ids.sort_unstable(); + Ok(AffiliateShortId::from( + used_ids + .iter() + .enumerate() + .find(|(index, assigned_id)| &AffiliateShortId::from(*index as u8) != *assigned_id) + .map(|(index, _)| index) + .unwrap_or(used_id_len) as u8, + )) + } +} + #[cfg(test)] mod tests { use super::*; @@ -810,4 +822,27 @@ mod tests { ); } } + + #[test] + fn test_find_lowest_unused_short_id() { + fn test_lowest(used_ids: &mut Vec, expected: AffiliateShortId) { + assert_eq!(find_lowest_unused_short_id(used_ids).unwrap(), expected); + assert_eq!( + used_ids.iter().find(|id| *id == &expected), + None, + "Should not overwrite existing IDs" + ); + used_ids.push(expected); + } + + let mut used_ids = vec![AffiliateShortId::from(1), AffiliateShortId::from(3)]; + test_lowest(&mut used_ids, AffiliateShortId::from(0)); + test_lowest(&mut used_ids, AffiliateShortId::from(2)); + test_lowest(&mut used_ids, AffiliateShortId::from(4)); + test_lowest(&mut used_ids, AffiliateShortId::from(5)); + let mut used_ids: Vec = + (0..u8::MAX).map(AffiliateShortId::from).collect(); + test_lowest(&mut used_ids, AffiliateShortId::from(255)); + assert!(find_lowest_unused_short_id(&used_ids).is_err()); + } } diff --git a/bouncer/shared/send_btc.ts b/bouncer/shared/send_btc.ts index e4c1442946..5f026cf607 100644 --- a/bouncer/shared/send_btc.ts +++ b/bouncer/shared/send_btc.ts @@ -36,7 +36,7 @@ export async function fundAndSendTransaction( } export async function sendVaultTransaction( - nulldataUtxo: string, + nulldataPayload: string, amountBtc: number, depositAddress: string, refundAddress: string, @@ -47,8 +47,7 @@ export async function sendVaultTransaction( [depositAddress]: amountBtc, }, { - // The `createRawTransaction` function will add the op codes, so we have to remove them here. - data: nulldataUtxo.replace('0x', '').substring(4), + data: nulldataPayload.replace('0x', ''), }, ], refundAddress, diff --git a/bouncer/tests/btc_vault_swap.ts b/bouncer/tests/btc_vault_swap.ts index b93ceb18da..a80cf72edb 100644 --- a/bouncer/tests/btc_vault_swap.ts +++ b/bouncer/tests/btc_vault_swap.ts @@ -22,7 +22,7 @@ const commissionBps = 100; interface VaultSwapDetails { chain: string; - nulldata_utxo: string; + nulldata_payload: string; deposit_address: string; } @@ -64,11 +64,11 @@ async function buildAndSendBtcVaultSwap( )) as unknown as VaultSwapDetails; assert.strictEqual(vaultSwapDetails.chain, 'Bitcoin'); - testBtcVaultSwap.debugLog('nulldata_utxo:', vaultSwapDetails.nulldata_utxo); + testBtcVaultSwap.debugLog('nulldata_payload:', vaultSwapDetails.nulldata_payload); testBtcVaultSwap.debugLog('deposit_address:', vaultSwapDetails.deposit_address); const txid = await sendVaultTransaction( - vaultSwapDetails.nulldata_utxo, + vaultSwapDetails.nulldata_payload, depositAmountBtc, vaultSwapDetails.deposit_address, refundAddress, diff --git a/engine/src/witness/btc/vault_swaps.rs b/engine/src/witness/btc/vault_swaps.rs index 8465f270ed..8fd7615859 100644 --- a/engine/src/witness/btc/vault_swaps.rs +++ b/engine/src/witness/btc/vault_swaps.rs @@ -180,7 +180,10 @@ mod tests { key::TweakedPublicKey, PubkeyHash, ScriptHash, WPubkeyHash, WScriptHash, }; - use cf_chains::address::EncodedAddress; + use cf_chains::{ + address::EncodedAddress, + btc::{BitcoinOp, BitcoinScript}, + }; use secp256k1::XOnlyPublicKey; use sp_core::bounded_vec; @@ -209,6 +212,16 @@ mod tests { }, }); + fn add_opcodes_to_data(data: Vec) -> ScriptBuf { + ScriptBuf::from_bytes( + BitcoinScript::new(&[ + BitcoinOp::Return, + BitcoinOp::PushBytes { bytes: data.try_into().unwrap() }, + ]) + .raw(), + ) + } + #[test] fn script_buf_to_script_pubkey_conversion() { // Check that we can convert from all types of bitcoin addresses: @@ -261,7 +274,6 @@ mod tests { let refund_pubkey = ScriptPubkey::P2PKH(REFUND_PK_HASH); let refund_script = ScriptBuf::new_p2pkh(&PubkeyHash::from_byte_array(REFUND_PK_HASH)); assert_eq!(refund_pubkey.bytes(), refund_script.to_bytes()); - let tx = fake_transaction( vec![ // A UTXO spending into our vault; @@ -274,9 +286,9 @@ mod tests { VerboseTxOut { value: Amount::from_sat(0), n: 1, - script_pubkey: ScriptBuf::from_bytes( - encode_swap_params_in_nulldata_utxo(MOCK_SWAP_PARAMS.clone()).raw(), - ), + script_pubkey: add_opcodes_to_data(encode_swap_params_in_nulldata_payload( + MOCK_SWAP_PARAMS.clone(), + )), }, // A UTXO containing refund address: VerboseTxOut { @@ -327,9 +339,7 @@ mod tests { #[test] fn extract_nulldata_utxo() { for data in [vec![0x3u8; 1_usize], vec![0x3u8; 75_usize], vec![0x3u8; 80_usize]] { - let script_buf = - ScriptBuf::from_bytes(encode_data_in_nulldata_utxo(&data).unwrap().raw()); - + let script_buf = add_opcodes_to_data(data.clone()); assert_eq!(try_extract_utxo_encoded_data(&script_buf), Some(&data[..])); } diff --git a/state-chain/chains/src/btc/vault_swap_encoding.rs b/state-chain/chains/src/btc/vault_swap_encoding.rs index f01bbdbb23..ff4cfb4042 100644 --- a/state-chain/chains/src/btc/vault_swap_encoding.rs +++ b/state-chain/chains/src/btc/vault_swap_encoding.rs @@ -6,8 +6,6 @@ use sp_core::ConstU32; use sp_runtime::BoundedVec; use sp_std::vec::Vec; -use super::{BitcoinOp, BitcoinScript}; - // The maximum length of data that can be encoded in a nulldata utxo const MAX_NULLDATA_LENGTH: usize = 80; const CURRENT_VERSION: u8 = 0; @@ -91,19 +89,8 @@ pub struct SharedCfParameters { pub affiliates: BoundedVec>, } -pub fn encode_data_in_nulldata_utxo(data: &[u8]) -> Option { - if data.len() > MAX_NULLDATA_LENGTH { - return None; - } - - Some(BitcoinScript::new(&[ - BitcoinOp::Return, - BitcoinOp::PushBytes { bytes: data.to_vec().try_into().expect("size checked just above") }, - ])) -} - -pub fn encode_swap_params_in_nulldata_utxo(params: UtxoEncodedData) -> BitcoinScript { - encode_data_in_nulldata_utxo(¶ms.encode()).expect("params must always fit in utxo") +pub fn encode_swap_params_in_nulldata_payload(params: UtxoEncodedData) -> Vec { + params.encode() } #[cfg(test)] diff --git a/state-chain/pallets/cf-swapping/src/lib.rs b/state-chain/pallets/cf-swapping/src/lib.rs index 86d59a064e..519ddb763b 100644 --- a/state-chain/pallets/cf-swapping/src/lib.rs +++ b/state-chain/pallets/cf-swapping/src/lib.rs @@ -757,8 +757,6 @@ pub mod pallet { AffiliateNotRegistered, /// Bitcoin vault swaps only support up to 2 affiliates. TooManyAffiliates, - /// No empty affiliate short id available. - NoEmptyAffiliateShortId, /// The Bonder does not have enough Funds to cover the bond. InsufficientFunds, } @@ -2510,7 +2508,6 @@ impl AffiliateRegistry for Pallet { AffiliateIdMapping::::get(broker_id, affiliate_short_id) } - /// This function iterates over a storage map. Only for use in rpc methods. /// This function iterates over a storage map. Only for use in rpc methods. fn get_short_id( broker_id: &Self::AccountId, diff --git a/state-chain/runtime/src/lib.rs b/state-chain/runtime/src/lib.rs index 4b28076264..c21b9f621e 100644 --- a/state-chain/runtime/src/lib.rs +++ b/state-chain/runtime/src/lib.rs @@ -47,7 +47,7 @@ use cf_chains::{ btc::{ api::BitcoinApi, vault_swap_encoding::{ - encode_swap_params_in_nulldata_utxo, SharedCfParameters, UtxoEncodedData, + encode_swap_params_in_nulldata_payload, SharedCfParameters, UtxoEncodedData, }, BitcoinCrypto, BitcoinRetryPolicy, ScriptPubkey, }, @@ -2215,7 +2215,7 @@ impl_runtime_apis! { .to_address(&Environment::network_environment().into()); Ok(VaultSwapDetails::Bitcoin { - nulldata_utxo: encode_swap_params_in_nulldata_utxo(params).raw(), + nulldata_payload: encode_swap_params_in_nulldata_payload(params), deposit_address, }) }, diff --git a/state-chain/runtime/src/runtime_apis.rs b/state-chain/runtime/src/runtime_apis.rs index 25da2b9e7a..48808c30c3 100644 --- a/state-chain/runtime/src/runtime_apis.rs +++ b/state-chain/runtime/src/runtime_apis.rs @@ -41,7 +41,7 @@ type VanityName = Vec; pub enum VaultSwapDetails { Bitcoin { #[serde(with = "sp_core::bytes")] - nulldata_utxo: Vec, + nulldata_payload: Vec, deposit_address: BtcAddress, }, } @@ -52,8 +52,8 @@ impl VaultSwapDetails { F: FnOnce(BtcAddress) -> T, { match self { - VaultSwapDetails::Bitcoin { nulldata_utxo, deposit_address } => - VaultSwapDetails::Bitcoin { nulldata_utxo, deposit_address: f(deposit_address) }, + VaultSwapDetails::Bitcoin { nulldata_payload, deposit_address } => + VaultSwapDetails::Bitcoin { nulldata_payload, deposit_address: f(deposit_address) }, } } }