refactor: remove op codes from btc vault swap encoding

api/lib/src/lib.rs

@@ -521,7 +521,7 @@ pub trait BrokerApi: SignedExtrinsicApi + StorageApi + Sized + Send + Sync + 'st
 		affiliate_id: AccountId32,
 		short_id: Option<AffiliateShortId>,
 	) -> Result<AffiliateShortId> {
-		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<AffiliateShortId> =
+			// Auto assign the lowest unused short id
+			let used_ids: Vec<AffiliateShortId> =
 				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<AffiliateShortId> {
+	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<AffiliateShortId>, 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<AffiliateShortId> =
+			(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());
+	}
 }

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,

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,

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<u8>) -> 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[..]));
 		}
 

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<AffiliateAndFee, ConstU32<MAX_AFFILIATES>>,
 }
 
-pub fn encode_data_in_nulldata_utxo(data: &[u8]) -> Option<BitcoinScript> {
-	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(&params.encode()).expect("params must always fit in utxo")
+pub fn encode_swap_params_in_nulldata_payload(params: UtxoEncodedData) -> Vec<u8> {
+	params.encode()
 }
 
 #[cfg(test)]

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<T: Config> AffiliateRegistry for Pallet<T> {
 		AffiliateIdMapping::<T>::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,

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,
 					})
 				},

state-chain/runtime/src/runtime_apis.rs

@@ -41,7 +41,7 @@ type VanityName = Vec<u8>;
 pub enum VaultSwapDetails<BtcAddress> {
 	Bitcoin {
 		#[serde(with = "sp_core::bytes")]
-		nulldata_utxo: Vec<u8>,
+		nulldata_payload: Vec<u8>,
 		deposit_address: BtcAddress,
 	},
 }
@@ -52,8 +52,8 @@ impl<BtcAddress> VaultSwapDetails<BtcAddress> {
 		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) },
 		}
 	}
 }