From 2f6c6e39c2d210ed1fe206330de324eaf07bb323 Mon Sep 17 00:00:00 2001 From: Ahmad Kaouk <56095276+ahmadkaouk@users.noreply.github.com> Date: Thu, 30 Oct 2025 12:19:14 +0100 Subject: [PATCH] fix: add explicit sovereign account balance check in unlock_tokens (#253) Add defensive validation to ensure the Ethereum sovereign account has sufficient balance before unlocking tokens. This addresses an audit finding where the lack of explicit balance checking created an unreliable security control that depended on implicit runtime behavior. Changes: - Add InsufficientSovereignBalance error variant for clear error messaging - Add explicit balance check in unlock_tokens before transfer - Update tests across all runtimes (testnet, stagenet, mainnet) to validate the specific error is returned when sovereign account has insufficient funds The explicit check provides better error messages that can propagate through the Ethereum bridge and makes debugging sovereign account balance issues easier. --- .../datahaven-native-transfer/src/lib.rs | 22 ++++++++++----- .../datahaven-native-transfer/src/tests.rs | 26 ++++++++++++++++-- .../mainnet/tests/native_token_transfer.rs | 7 +++-- .../stagenet/tests/native_token_transfer.rs | 7 +++-- .../testnet/tests/native_token_transfer.rs | 7 +++-- test/.papi/descriptors/package.json | 2 +- test/.papi/metadata/datahaven.scale | Bin 620948 -> 621031 bytes 7 files changed, 51 insertions(+), 20 deletions(-) diff --git a/operator/pallets/datahaven-native-transfer/src/lib.rs b/operator/pallets/datahaven-native-transfer/src/lib.rs index 11069a43..01f2f998 100644 --- a/operator/pallets/datahaven-native-transfer/src/lib.rs +++ b/operator/pallets/datahaven-native-transfer/src/lib.rs @@ -26,7 +26,7 @@ use frame_support::{ use snowbridge_core::TokenId; use snowbridge_outbound_queue_primitives::v2::{Command, Message as OutboundMessage, SendMessage}; use sp_core::{H160, H256}; -use sp_runtime::BoundedVec; +use sp_runtime::{traits::Saturating, BoundedVec}; use sp_std::vec; pub use pallet::*; @@ -135,6 +135,8 @@ pub mod pallet { ZeroFee, /// Native token has not been registered on Ethereum yet TokenNotRegistered, + /// Insufficient balance in Ethereum sovereign account + InsufficientSovereignBalance, } #[pallet::call] @@ -275,13 +277,19 @@ pub mod pallet { /// /// Transfers tokens from the Ethereum sovereign account back to user pub fn unlock_tokens(who: &T::AccountId, amount: BalanceOf) -> DispatchResult { + let sovereign = T::EthereumSovereignAccount::get(); + let balance = T::Currency::balance(&sovereign); + let minimum_balance = T::Currency::minimum_balance(); + let available_balance = balance.saturating_sub(minimum_balance); + + // Allow unlocking only from funds that exceed the existential buffer. + ensure!( + available_balance >= amount, + Error::::InsufficientSovereignBalance + ); + // Transfer from the Ethereum sovereign account - T::Currency::transfer( - &T::EthereumSovereignAccount::get(), - who, - amount, - Preservation::Preserve, - )?; + T::Currency::transfer(&sovereign, who, amount, Preservation::Preserve)?; Self::deposit_event(Event::TokensUnlocked { account: who.clone(), diff --git a/operator/pallets/datahaven-native-transfer/src/tests.rs b/operator/pallets/datahaven-native-transfer/src/tests.rs index 6ccad384..da460c3b 100644 --- a/operator/pallets/datahaven-native-transfer/src/tests.rs +++ b/operator/pallets/datahaven-native-transfer/src/tests.rs @@ -243,7 +243,10 @@ fn unlock_tokens_works() { )); assert_eq!(Balances::balance(&BOB), INITIAL_BALANCE + unlock_amount); - assert_eq!(Balances::balance(ÐEREUM_SOVEREIGN), 1); // Existential deposit remains + assert_eq!( + Balances::balance(ÐEREUM_SOVEREIGN), + Balances::minimum_balance() + ); // Existential deposit remains // Check event assert_eq!( @@ -262,7 +265,21 @@ fn unlock_insufficient_sovereign_balance_fails() { // Try to unlock without any locked tokens assert_noop!( DataHavenNativeTransfer::::unlock_tokens(&BOB, 1000), - DispatchError::Token(sp_runtime::TokenError::FundsUnavailable) + Error::::InsufficientSovereignBalance + ); + }); +} + +#[test] +fn unlock_fails_if_existential_deposit_would_be_consumed() { + new_test_ext().execute_with(|| { + let amount = 10u128; + assert_ok!(DataHavenNativeTransfer::::lock_tokens(&ALICE, amount)); + + // Attempt to withdraw the full sovereign balance, which should leave the account below ED + assert_noop!( + DataHavenNativeTransfer::::unlock_tokens(&BOB, amount), + Error::::InsufficientSovereignBalance ); }); } @@ -283,7 +300,10 @@ fn lock_unlock_different_amounts() { &CHARLIE, 2999 )); - assert_eq!(Balances::balance(ÐEREUM_SOVEREIGN), 1); // Existential deposit remains + assert_eq!( + Balances::balance(ÐEREUM_SOVEREIGN), + Balances::minimum_balance() + ); // Existential deposit remains assert_eq!(Balances::balance(&BOB), INITIAL_BALANCE + 2000); assert_eq!(Balances::balance(&CHARLIE), INITIAL_BALANCE + 2999); }); diff --git a/operator/runtime/mainnet/tests/native_token_transfer.rs b/operator/runtime/mainnet/tests/native_token_transfer.rs index 407f83dd..7739b282 100644 --- a/operator/runtime/mainnet/tests/native_token_transfer.rs +++ b/operator/runtime/mainnet/tests/native_token_transfer.rs @@ -374,9 +374,10 @@ fn processing_fails_with_insufficient_sovereign_balance() { setup_sovereign_balance(TRANSFER_AMOUNT / 2); // Insufficient balance - let result = - snowbridge_pallet_inbound_queue_v2::Pallet::::process_message(alice, message); - assert!(result.is_err()); + assert_noop!( + snowbridge_pallet_inbound_queue_v2::Pallet::::process_message(alice, message), + pallet_datahaven_native_transfer::Error::::InsufficientSovereignBalance + ); }); } diff --git a/operator/runtime/stagenet/tests/native_token_transfer.rs b/operator/runtime/stagenet/tests/native_token_transfer.rs index de352f6c..6b96bd88 100644 --- a/operator/runtime/stagenet/tests/native_token_transfer.rs +++ b/operator/runtime/stagenet/tests/native_token_transfer.rs @@ -361,9 +361,10 @@ fn processing_fails_with_insufficient_sovereign_balance() { setup_sovereign_balance(TRANSFER_AMOUNT / 2); // Insufficient balance - let result = - snowbridge_pallet_inbound_queue_v2::Pallet::::process_message(alice, message); - assert!(result.is_err()); + assert_noop!( + snowbridge_pallet_inbound_queue_v2::Pallet::::process_message(alice, message), + pallet_datahaven_native_transfer::Error::::InsufficientSovereignBalance + ); }); } diff --git a/operator/runtime/testnet/tests/native_token_transfer.rs b/operator/runtime/testnet/tests/native_token_transfer.rs index e02fe707..6d5b4234 100644 --- a/operator/runtime/testnet/tests/native_token_transfer.rs +++ b/operator/runtime/testnet/tests/native_token_transfer.rs @@ -361,9 +361,10 @@ fn processing_fails_with_insufficient_sovereign_balance() { setup_sovereign_balance(TRANSFER_AMOUNT / 2); // Insufficient balance - let result = - snowbridge_pallet_inbound_queue_v2::Pallet::::process_message(alice, message); - assert!(result.is_err()); + assert_noop!( + snowbridge_pallet_inbound_queue_v2::Pallet::::process_message(alice, message), + pallet_datahaven_native_transfer::Error::::InsufficientSovereignBalance + ); }); } diff --git a/test/.papi/descriptors/package.json b/test/.papi/descriptors/package.json index 5e04bf6c..f91b756b 100644 --- a/test/.papi/descriptors/package.json +++ b/test/.papi/descriptors/package.json @@ -1,5 +1,5 @@ { - "version": "0.1.0-autogenerated.12568285267841290320", + "version": "0.1.0-autogenerated.2256885919410986602", "name": "@polkadot-api/descriptors", "files": [ "dist" diff --git a/test/.papi/metadata/datahaven.scale b/test/.papi/metadata/datahaven.scale index 2ea69d167bcc542528f4cda5087b32180dc4c38d..4d9188cb45be272e52dc6c5a1ce4257cbfb1dfff 100644 GIT binary patch delta 135 zcmbR8S@rp6)rJFEi69|F{*6;b%>>xF}T1puedZVEi*YYHLoN%zbv&VH8VZW zDKRH8FFBQggXILWyh0L0Tp=@0!L=j1)rJFEi69|F)D2Tb%>>xacUupQv07HEI`Z(#B4y!zWvV;j@MfN DdVU$x