Audit Issue 5: Use reward address (if set) for reward allocation in the Outbound Queue v2

bridges/snowbridge/pallets/outbound-queue-v2/src/lib.rs

@@ -429,8 +429,17 @@ pub mod pallet {
 			let order = <PendingOrders<T>>::get(nonce).ok_or(Error::<T>::InvalidPendingNonce)?;
 
 			if order.fee > 0 {
+				// Use the reward_address from the receipt if valid, otherwise fall back to the
+				// relayer
+				let reward_account = Self::decode_reward_address(&receipt.reward_address)
+					.unwrap_or_else(|| relayer.clone());
+
 				// Pay relayer reward
-				T::RewardPayment::register_reward(&relayer, T::DefaultRewardKind::get(), order.fee);
+				T::RewardPayment::register_reward(
+					&reward_account,
+					T::DefaultRewardKind::get(),
+					order.fee,
+				);
 			}
 
 			<PendingOrders<T>>::remove(nonce);
@@ -439,5 +448,18 @@ pub mod pallet {
 
 			Ok(())
 		}
+
+		/// Helper method to decode a reward address into an AccountId
+		fn decode_reward_address(
+			reward_address: &[u8; 32],
+		) -> Option<<T as frame_system::Config>::AccountId> {
+			// Zero address should fall back to the relayer account
+			if reward_address.iter().all(|&b| b == 0) {
+				return None;
+			}
+
+			// Try to decode the account ID from the reward address
+			<T as frame_system::Config>::AccountId::decode(&mut &reward_address[..]).ok()
+		}
 	}
 }

bridges/snowbridge/pallets/outbound-queue-v2/src/mock.rs

@@ -119,12 +119,19 @@ pub enum BridgeReward {
 	Snowbridge,
 }
 
+parameter_types! {
+	pub static RegisteredRewardsCount: u128 = 0;
+	pub static RegisteredRewardsRelayer: <mock::Test as frame_system::Config>::AccountId = [0u8; 32].into();
+}
+
 impl RewardLedger<<mock::Test as frame_system::Config>::AccountId, BridgeReward, u128> for () {
 	fn register_reward(
-		_relayer: &<mock::Test as frame_system::Config>::AccountId,
+		relayer: &<mock::Test as frame_system::Config>::AccountId,
 		_reward: BridgeReward,
 		_reward_balance: u128,
 	) {
+		RegisteredRewardsCount::set(RegisteredRewardsCount::get().saturating_add(1));
+		RegisteredRewardsRelayer::set(relayer.clone());
 	}
 }
 

bridges/snowbridge/pallets/outbound-queue-v2/src/test.rs

@@ -16,6 +16,7 @@ use snowbridge_outbound_queue_primitives::{
 	SendError,
 };
 use sp_core::{hexdisplay::HexDisplay, H256};
+use sp_runtime::AccountId32;
 
 #[test]
 fn submit_messages_and_commit() {
@@ -308,3 +309,54 @@ fn encode_register_pna() {
 	println!("{}", HexDisplay::from(&message_abi_encoded));
 	assert_eq!(hex!("000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000003e80000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000000000000000000000000000000124f80000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000000e000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000000000000000000000000000000000000000000c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000").to_vec(), message_abi_encoded)
 }
+
+#[test]
+fn process_delivery_receipt_with_reward_address() {
+	new_tester().execute_with(|| {
+		let relayer_account: AccountId32 = AccountId32::new([1u8; 32]);
+		let beneficiary_account: AccountId32 = AccountId32::new([2u8; 32]);
+
+		let nonce = 123u64;
+		let fee = 1000u128;
+
+		let create_order = || PendingOrder { nonce, fee, block_number: System::block_number() };
+
+		<PendingOrders<Test>>::insert(nonce, create_order());
+
+		let mut receipt = DeliveryReceipt {
+			gateway: GatewayAddress::get(),
+			nonce,
+			topic: H256::zero(),
+			success: true,
+			reward_address: [0u8; 32], // Zero address, should fall back to relayer
+		};
+
+		let result1 =
+			OutboundQueue::process_delivery_receipt(relayer_account.clone(), receipt.clone());
+		assert_ok!(result1);
+
+		assert_eq!(RegisteredRewardsCount::get(), 1, "Relayer reward should have been registered");
+		assert_eq!(
+			RegisteredRewardsRelayer::get(),
+			relayer_account.clone(),
+			"Relayer account should have been used to register for rewards"
+		);
+
+		<PendingOrders<Test>>::insert(nonce, create_order());
+
+		let mut beneficiary_bytes = [0u8; 32];
+		beneficiary_bytes.copy_from_slice(&beneficiary_account.encode());
+		receipt.reward_address = beneficiary_bytes; // Then use the reward address
+
+		let result2 =
+			OutboundQueue::process_delivery_receipt(relayer_account.clone(), receipt.clone());
+		assert_ok!(result2);
+
+		assert_eq!(RegisteredRewardsCount::get(), 2, "Relayer reward should have been registered");
+		assert_eq!(
+			RegisteredRewardsRelayer::get(),
+			receipt.reward_address.into(),
+			"Reward address should have been used to register for rewards"
+		);
+	});
+}