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.
This commit is contained in:
Ahmad Kaouk 2025-10-30 12:19:14 +01:00 committed by GitHub
parent 839fe6399f
commit 2f6c6e39c2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 51 additions and 20 deletions

View file

@ -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<T>) -> 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::<T>::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(),

View file

@ -243,7 +243,10 @@ fn unlock_tokens_works() {
));
assert_eq!(Balances::balance(&BOB), INITIAL_BALANCE + unlock_amount);
assert_eq!(Balances::balance(&ETHEREUM_SOVEREIGN), 1); // Existential deposit remains
assert_eq!(
Balances::balance(&ETHEREUM_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::<Test>::unlock_tokens(&BOB, 1000),
DispatchError::Token(sp_runtime::TokenError::FundsUnavailable)
Error::<Test>::InsufficientSovereignBalance
);
});
}
#[test]
fn unlock_fails_if_existential_deposit_would_be_consumed() {
new_test_ext().execute_with(|| {
let amount = 10u128;
assert_ok!(DataHavenNativeTransfer::<Test>::lock_tokens(&ALICE, amount));
// Attempt to withdraw the full sovereign balance, which should leave the account below ED
assert_noop!(
DataHavenNativeTransfer::<Test>::unlock_tokens(&BOB, amount),
Error::<Test>::InsufficientSovereignBalance
);
});
}
@ -283,7 +300,10 @@ fn lock_unlock_different_amounts() {
&CHARLIE, 2999
));
assert_eq!(Balances::balance(&ETHEREUM_SOVEREIGN), 1); // Existential deposit remains
assert_eq!(
Balances::balance(&ETHEREUM_SOVEREIGN),
Balances::minimum_balance()
); // Existential deposit remains
assert_eq!(Balances::balance(&BOB), INITIAL_BALANCE + 2000);
assert_eq!(Balances::balance(&CHARLIE), INITIAL_BALANCE + 2999);
});

View file

@ -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::<Runtime>::process_message(alice, message);
assert!(result.is_err());
assert_noop!(
snowbridge_pallet_inbound_queue_v2::Pallet::<Runtime>::process_message(alice, message),
pallet_datahaven_native_transfer::Error::<Runtime>::InsufficientSovereignBalance
);
});
}

View file

@ -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::<Runtime>::process_message(alice, message);
assert!(result.is_err());
assert_noop!(
snowbridge_pallet_inbound_queue_v2::Pallet::<Runtime>::process_message(alice, message),
pallet_datahaven_native_transfer::Error::<Runtime>::InsufficientSovereignBalance
);
});
}

View file

@ -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::<Runtime>::process_message(alice, message);
assert!(result.is_err());
assert_noop!(
snowbridge_pallet_inbound_queue_v2::Pallet::<Runtime>::process_message(alice, message),
pallet_datahaven_native_transfer::Error::<Runtime>::InsufficientSovereignBalance
);
});
}

View file

@ -1,5 +1,5 @@
{
"version": "0.1.0-autogenerated.12568285267841290320",
"version": "0.1.0-autogenerated.2256885919410986602",
"name": "@polkadot-api/descriptors",
"files": [
"dist"

Binary file not shown.