Skip to content

Fix/cross chain signer removal 295 - #343

Merged
Dev-Zully merged 2 commits into
OdyxeeeLabs:mainfrom
rabsqueen:fix/cross-chain-signer-removal-295
Aug 26, 2026
Merged

Fix/cross chain signer removal 295#343
Dev-Zully merged 2 commits into
OdyxeeeLabs:mainfrom
rabsqueen:fix/cross-chain-signer-removal-295

Conversation

@rabsqueen

@rabsqueen rabsqueen commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a high-priority logic error in cross_chain_verifier::remove_authorized_signer.

The function previously contained two nearly identical signer-removal blocks. Both blocks checked signer existence, removed the signer from storage, and decremented SignerCount, causing the count to be decremented twice for a single removal and potentially underflowing the u32 counter.

What Changed

🐛 Removed Duplicate Removal Logic

  • Removed the redundant signer-removal block.
  • Signer existence is now checked once.
  • The authorized signer is removed from storage exactly once.
  • SignerCount is updated exactly once per successful signer removal.

🔢 Added Underflow Protection

Preserved/enforced the counter guard before decrementing:

if count > 0 {
    count - 1
}

This prevents SignerCount from wrapping when the counter is already zero.

Before

remove_authorized_signer()
        ↓
Check signer exists
        ↓
Remove signer
        ↓
SignerCount -= 1
        ↓
Duplicate block
        ↓
Check signer exists again
        ↓
Remove signer again
        ↓
SignerCount -= 1 ❌

A single signer removal could therefore decrement the counter twice.

After

remove_authorized_signer()
        ↓
Check signer exists
        ↓
Remove signer once
        ↓
if count > 0
        ↓
SignerCount -= 1

The counter now accurately represents the number of authorized signers.

Validation

Verified that:

  • Duplicate signer-removal logic has been removed.
  • A signer is removed from storage exactly once.
  • SignerCount is decremented exactly once per successful removal.
  • Counter underflow is prevented.
  • Existing signer-existence validation is preserved.
  • Removing an authorized signer does not cause SignerCount to wrap.

Acceptance Criteria

  • Remove the duplicate removal block.
  • Check signer existence once.
  • Remove the signer exactly once.
  • Decrement SignerCount exactly once.
  • Protect against counter underflow with count > 0.
  • Preserve existing authorization and storage behaviour.

Result

remove_authorized_signer now performs a single, consistent signer-removal operation and maintains an accurate SignerCount without the risk of u32 underflow or counter corruption.

Closes #295

@drips-wave

drips-wave Bot commented Aug 25, 2026

Copy link
Copy Markdown

@rabsqueen Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Dev-Zully

Copy link
Copy Markdown
Contributor

@rabsqueen resolve conflit

@Dev-Zully
Dev-Zully merged commit a167199 into OdyxeeeLabs:main Aug 26, 2026
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.

CT-008: cross_chain_verifier remove_authorized_signer checks signer existence twice and decrements count twice

2 participants