Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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?

Copy link
Copy Markdown
Contributor

@yrong yrong May 8, 2025

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
Loading