Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 6 additions & 5 deletions bridges/snowbridge/pallets/inbound-queue-v2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ pub mod pallet {
/// StorageMap used for encoding a SparseBitmapImpl that tracks whether a specific nonce has
/// been processed or not. Message nonces are unique and never repeated.
#[pallet::storage]
pub type NonceBitmap<T: Config> = StorageMap<_, Twox64Concat, u128, u128, ValueQuery>;
pub type NonceBitmap<T: Config> = StorageMap<_, Twox64Concat, u64, u128, ValueQuery>;

/// The current operating mode of the pallet.
#[pallet::storage]
Expand Down Expand Up @@ -232,13 +232,17 @@ pub mod pallet {
let (nonce, relayer_fee) = (message.nonce, message.relayer_fee);

// Verify the message has not been processed
ensure!(!Nonce::<T>::get(nonce.into()), Error::<T>::InvalidNonce);
ensure!(!Nonce::<T>::get(nonce), Error::<T>::InvalidNonce);

let xcm =
T::MessageConverter::convert(message).map_err(|error| Error::<T>::from(error))?;

// Forward XCM to AH
let dest = Location::new(1, [Parachain(T::AssetHubParaId::get())]);

// Mark message as received
Nonce::<T>::set(nonce);

let message_id =
Self::send_xcm(dest.clone(), &relayer, xcm.clone()).map_err(|error| {
tracing::error!(target: LOG_TARGET, ?error, ?dest, ?xcm, "XCM send failed with error");
Expand All @@ -254,9 +258,6 @@ pub mod pallet {
);
}

// Mark message as received
Nonce::<T>::set(nonce.into());

Self::deposit_event(Event::MessageReceived { nonce, message_id });

Ok(())
Expand Down
3 changes: 3 additions & 0 deletions bridges/snowbridge/pallets/inbound-queue-v2/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ parameter_types! {
pub UniversalLocation: InteriorLocation =
[GlobalConsensus(ByGenesis(WESTEND_GENESIS_HASH)), Parachain(1002)].into();
pub AssetHubFromEthereum: Location = Location::new(1,[GlobalConsensus(ByGenesis(WESTEND_GENESIS_HASH)),Parachain(1000)]);
pub AssetHubUniversalLocation: InteriorLocation = [GlobalConsensus(ByGenesis(WESTEND_GENESIS_HASH)), Parachain(1000)].into();
pub SnowbridgeReward: BridgeReward = BridgeReward::Snowbridge;
pub const CreateAssetCall: [u8;2] = [53, 0];
pub const CreateAssetDeposit: u128 = 10_000_000_000u128;
Expand Down Expand Up @@ -155,6 +156,8 @@ impl inbound_queue_v2::Config for Test {
GatewayAddress,
UniversalLocation,
AssetHubFromEthereum,
AssetHubUniversalLocation,
AccountId,
>;
#[cfg(feature = "runtime-benchmarks")]
type Helper = Test;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::Pallet as OutboundQueue;
#[benchmarks(
where
<T as Config>::MaxMessagePayloadSize: Get<u32>,
<T as frame_system::Config>::AccountId: From<[u8; 32]>,
)]
mod benchmarks {
use super::*;
Expand Down
51 changes: 36 additions & 15 deletions bridges/snowbridge/pallets/outbound-queue-v2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,13 +275,19 @@ pub mod pallet {
}

#[pallet::call]
impl<T: Config> Pallet<T> {
impl<T: Config> Pallet<T>
where
<T as frame_system::Config>::AccountId: From<[u8; 32]>,
{
#[pallet::call_index(1)]
#[pallet::weight(T::WeightInfo::submit_delivery_receipt())]
pub fn submit_delivery_receipt(
origin: OriginFor<T>,
event: Box<EventProof>,
) -> DispatchResult {
) -> DispatchResult
where
<T as frame_system::Config>::AccountId: From<[u8; 32]>,
{
let relayer = ensure_signed(origin)?;

// submit message to verifier for verification
Expand Down Expand Up @@ -314,7 +320,8 @@ pub mod pallet {
Self::deposit_event(Event::MessagesCommitted { root, count });
}

/// Process a message delivered by the MessageQueue pallet
/// Process a message delivered by the MessageQueue pallet.
/// IMPORTANT!! This method does not roll back storage changes on error.
pub(crate) fn do_process_message(
_: ProcessMessageOriginOf<T>,
mut message: &[u8],
Expand All @@ -332,8 +339,6 @@ pub mod pallet {
return Err(Yield);
}

let nonce = Nonce::<T>::get();

// Decode bytes into Message
let Message { origin, id, fee, commands } =
Message::decode(&mut message).map_err(|_| {
Expand All @@ -354,6 +359,16 @@ pub mod pallet {
payload: command.abi_encode(),
})
.collect();

let nonce = <Nonce<T>>::get().checked_add(1).ok_or_else(|| {
Self::deposit_event(Event::MessageRejected {
id: None,
payload: message.to_vec(),
error: Unsupported,
});
Unsupported
})?;

let outbound_message = OutboundMessage {
origin,
nonce,
Expand Down Expand Up @@ -402,14 +417,7 @@ pub mod pallet {
};
<PendingOrders<T>>::insert(nonce, order);

Nonce::<T>::set(nonce.checked_add(1).ok_or_else(|| {
Self::deposit_event(Event::MessageRejected {
id: Some(id),
payload: message.to_vec(),
error: Unsupported,
});
Unsupported
})?);
<Nonce<T>>::set(nonce);

Self::deposit_event(Event::MessageAccepted { id, nonce });

Expand All @@ -420,17 +428,30 @@ pub mod pallet {
pub fn process_delivery_receipt(
relayer: <T as frame_system::Config>::AccountId,
receipt: DeliveryReceipt,
) -> DispatchResult {
) -> DispatchResult
where
<T as frame_system::Config>::AccountId: From<[u8; 32]>,
{
// Verify that the message was submitted from the known Gateway contract
ensure!(T::GatewayAddress::get() == receipt.gateway, Error::<T>::InvalidGateway);

let reward_account = if receipt.reward_address == [0u8; 32] {
relayer
} else {
receipt.reward_address.into()
};

let nonce = receipt.nonce;

let order = <PendingOrders<T>>::get(nonce).ok_or(Error::<T>::InvalidPendingNonce)?;

if order.fee > 0 {
// 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);
Expand Down
9 changes: 2 additions & 7 deletions bridges/snowbridge/pallets/outbound-queue-v2/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,10 @@ fn process_message_fails_on_max_nonce_reached() {

match last_event {
mock::RuntimeEvent::OutboundQueue(Event::MessageRejected {
id: Some(id),
id: None,
payload: _,
error: ProcessMessageError::Unsupported,
}) => {
assert_eq!(
id,
hex!("0000000000000000000000000000000000000000000000000000000000000001").into()
);
},
}) => {},
_ => {
panic!("Expected Event::MessageRejected(Unsupported) but got {:?}", last_event);
},
Expand Down
2 changes: 1 addition & 1 deletion bridges/snowbridge/pallets/system-frontend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub enum BridgeHubRuntime {
/// Call indices for dispatchables within `snowbridge-pallet-system-v2`
#[derive(Encode, Decode, Debug, PartialEq, Clone, TypeInfo)]
pub enum EthereumSystemCall {
#[codec(index = 0)]
#[codec(index = 2)]
RegisterToken {
sender: Box<VersionedLocation>,
asset_id: Box<VersionedLocation>,
Expand Down
13 changes: 5 additions & 8 deletions bridges/snowbridge/pallets/system-v2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ pub mod pallet {
/// - `impl_address`: The address of the implementation contract.
/// - `impl_code_hash`: The codehash of the implementation contract.
/// - `initializer`: Optionally call an initializer on the implementation contract.
#[pallet::call_index(3)]
#[pallet::call_index(0)]
#[pallet::weight((<T as pallet::Config>::WeightInfo::upgrade(), DispatchClass::Operational))]
pub fn upgrade(
origin: OriginFor<T>,
Expand Down Expand Up @@ -159,7 +159,7 @@ pub mod pallet {
/// Fee required: No
///
/// - `origin`: Must be `GovernanceOrigin`
#[pallet::call_index(4)]
#[pallet::call_index(1)]
#[pallet::weight((<T as pallet::Config>::WeightInfo::set_operating_mode(), DispatchClass::Operational))]
pub fn set_operating_mode(origin: OriginFor<T>, mode: OperatingMode) -> DispatchResult {
let origin_location = T::GovernanceOrigin::ensure_origin(origin)?;
Expand All @@ -179,8 +179,7 @@ pub mod pallet {
/// - `sender`: The original sender initiating the call on AH
/// - `asset_id`: Location of the asset (relative to this chain)
/// - `metadata`: Metadata to include in the instantiated ERC20 contract on Ethereum
/// - `fee`: Ether to pay for the execution cost on Ethereum
#[pallet::call_index(0)]
#[pallet::call_index(2)]
#[pallet::weight(<T as pallet::Config>::WeightInfo::register_token())]
pub fn register_token(
origin: OriginFor<T>,
Expand Down Expand Up @@ -226,14 +225,12 @@ pub mod pallet {
impl<T: Config> Pallet<T> {
/// Send `command` to the Gateway from a specific origin/agent
fn send(origin: H256, command: Command, fee: u128) -> DispatchResult {
let mut message = Message {
let message = Message {
origin,
id: Default::default(),
id: frame_system::unique((origin, &command, fee)).into(),
Comment on lines +228 to +230

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd assume the ID should be generated by the system-frontend pallet on AH here, along with an additional setTopic instruction using a provided ID.

Another issue is that the current register_token call is free, which may not be the intended behavior.

To address this, we could consider introducing a higher-level extrinsic in the system-v2 pallet, and have system-frontend call that entry point - passing in the necessary context such as the fee (for burning) and topic_id (for tracing). This wrapper would internally call functions like register_token or add_tip, as seen in PR #8271.

This isn’t specific to this backport PR and is non-blocking, just my two cents; we can revisit and improve it later.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ron can you open a ticket to track this?

@yrong yrong May 8, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fee,
commands: BoundedVec::try_from(vec![command]).unwrap(),
};
let hash = sp_io::hashing::blake2_256(&message.encode());
message.id = hash.into();

let ticket = <T as pallet::Config>::OutboundQueue::validate(&message)
.map_err(|err| Error::<T>::Send(err))?;
Expand Down
Loading