Skip to content

Audit Issue 21: Better unique message ID in Outbound Queue v2#21

Merged
claravanstaden merged 3 commits intosnowbridge-v2-audit-fixesfrom
sv2-audit-issue-21
Apr 14, 2025
Merged

Audit Issue 21: Better unique message ID in Outbound Queue v2#21
claravanstaden merged 3 commits intosnowbridge-v2-audit-fixesfrom
sv2-audit-issue-21

Conversation

@claravanstaden
Copy link
Copy Markdown
Owner

Resolves: SNO-1474

let mut message = Message {
origin,
id: Default::default(),
id: frame_system::unique((origin, command, fee)),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd suggest using blake2_256 for determinism.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Addressed in 78b7326.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't think determinism is useful in this case. These are governance messages emitted by BridgeHub itself. I don't think any UIs are going to try and deterministically recreate these message ids.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Reverted 78b7326.

Copy link
Copy Markdown

@yrong yrong left a comment

Choose a reason for hiding this comment

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

+1

This reverts commit 78b7326.
@github-actions github-actions bot requested a review from yrong April 11, 2025 12:26
@github-actions
Copy link
Copy Markdown

Review required! Latest push from author must always be reviewed

@claravanstaden claravanstaden merged commit 38f1aee into snowbridge-v2-audit-fixes Apr 14, 2025
116 of 210 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants