Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Snowbridge - Various minor improvements #8020

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Snowbridge - Various minor improvements #8020

wants to merge 11 commits into from

Conversation

claravanstaden
Copy link
Contributor

@claravanstaden claravanstaden commented Mar 25, 2025 •
edited
Loading

  • Renames event in the Outbound Queue v2: Resolves SNO-1430
  • Remove unnecessary PaidFees event in Inbound Queue: Resolves SNO-1437
  • Remove unused types from Inbound Queue v2 pallet config: Resolves SNO-1436
  • Zero relayer reward in Inbound Queue should not be registered for a reward: Resolves SNO-1435
  • Moves Snowbridge System Frontend Pallet index from 80 to 36 on AH: Snowbridge V2 #7402 (comment)
  • Add message rejected event to outbound queue: Resolves SNO-1428 Closes [Snowbridge v2] emit Event::MessageRejected #7972
  • Removes unused method from Outbound Queue v2: Resolves SNO-1431
  • Fixes bug introduced in V2 related to commands on Ethereum's indexes: Introduced in Snowbridge V2 #7402, the command IDs need to match on the contract side. This ensures that when we upgrade BH with the new code, it will remain compatible with the existing contracts on Ethereum that don't know anything about V2.
  • Benchmarks submit_delivery_receipt in the Outbound Queue v2
  • Allow transact without transfer of any asset

@paritytech-review-bot paritytech-review-bot bot requested a review from a team March 25, 2025 09:59
@acatangiu acatangiu added the R0-silent Changes should not be mentioned in any release notes label Mar 25, 2025
@acatangiu acatangiu requested a review from a team March 25, 2025 11:38
@acatangiu
Copy link
Contributor

maybe some of these small PRs can be batched together for easier backporting

@acatangiu acatangiu enabled auto-merge March 25, 2025 11:44
@acatangiu acatangiu added the A4-backport-stable2503 Pull request must be backported to the stable2503 release branch label Mar 25, 2025
@claravanstaden claravanstaden changed the title Snowbridge - Renames event in the Outbound Queue v2 Snowbridge - Various minor improvements Mar 26, 2025
auto-merge was automatically disabled March 26, 2025 06:37

Head branch was pushed to by a user without write access

This was referenced Mar 26, 2025
Closed
Closed
Closed
serban300
serban300 previously approved these changes Mar 26, 2025
View reviewed changes
This was referenced Mar 26, 2025
Closed
Closed
acatangiu
acatangiu reviewed Mar 26, 2025
View reviewed changes
bridges/snowbridge/pallets/outbound-queue-v2/src/process_message_impl.rs Outdated
Comment on lines 21 to 24
Self::deposit_event(Event::MessageRejected {
id: None,
payload: Some(message.to_vec()),
error: ProcessMessageError::Overweight(weight),
});
Copy link
Contributor

Choose a reason for hiding this comment

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

see convo here #8015 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in ed489d1.

@claravanstaden claravanstaden requested review from a team as code owners March 26, 2025 10:10
yrong
yrong approved these changes Mar 27, 2025
View reviewed changes
Copy link
Contributor

@yrong yrong left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@claravanstaden I made claravanstaden#7 to fix the pallet index. Otherwise, everything looks good!

@yrong
Copy link
Contributor

yrong commented Mar 27, 2025 •
edited
Loading

Meanwhile, I've created claravanstaden#8, which includes my two previous PRs. It has been reviewed by Adrian but hasn’t received enough approvals yet.

I think it might be better for reviewers to review this single PR instead of reviewing them separately. @claravanstaden @acatangiu If you agree, I'll close the original PRs.

#8026
#8030

@yrong
Fix for CI (#7)
bbebe36 
* Fix format

* Update frontend pallet index

* Fix pallet index
@claravanstaden claravanstaden mentioned this pull request Mar 27, 2025
Merged
@acatangiu
Copy link
Contributor

Meanwhile, I've created claravanstaden#8, which includes my two previous PRs. It has been reviewed by Adrian but hasn’t received enough approvals yet.

I think it might be better for reviewers to review this single PR instead of reviewing them separately. @claravanstaden @acatangiu If you agree, I'll close the original PRs.

#8026 #8030

yes we can review everything here

@yrong
Merge other PRs (#8)
8003e31 
* Fix format

* Update frontend pallet index

* Fix pallet index

* Improve benchmarks

* Allow transact without transfer of any asset
@claravanstaden
Copy link
Contributor Author

@acatangiu @yrong all the changes are merged into this PR.

@serban300 serban300 dismissed their stale review March 27, 2025 10:23

More changes were added

This was referenced Mar 27, 2025
Closed
Closed
@yrong
Copy link
Contributor

yrong commented Mar 28, 2025

@claravanstaden Just one minor fix for clippy error in claravanstaden#9

@acatangiu acatangiu added this pull request to the merge queue Mar 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 28, 2025
@claravanstaden
Copy link
Contributor Author

@acatangiu not sure which CI is giving problems, maybe this one? https://github.com/paritytech/polkadot-sdk/actions/runs/14127396204/job/39579525628?pr=8020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vgeddes vgeddes vgeddes approved these changes

@yrong yrong yrong approved these changes

@acatangiu acatangiu acatangiu approved these changes

@raymondkfcheung raymondkfcheung raymondkfcheung approved these changes

@serban300 serban300 serban300 left review comments

Assignees
No one assigned
Labels
A4-backport-stable2503 Pull request must be backported to the stable2503 release branch R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Snowbridge v2] emit Event::MessageRejected
6 participants
@claravanstaden @acatangiu @yrong @vgeddes @serban300 @raymondkfcheung