Skip to content

SwapBugFixes#488

Merged
lydiagarms merged 10 commits intomasterfrom
lyd/swapELBug
Apr 23, 2026
Merged

SwapBugFixes#488
lydiagarms merged 10 commits intomasterfrom
lyd/swapELBug

Conversation

@lydiagarms
Copy link
Copy Markdown
Collaborator

@lydiagarms lydiagarms commented Apr 17, 2026

Summary

This PR addresses several issues: an event listener bug fix for Swap.zol, insufficient checks being included in the test contracts when re-initialisable contracts are used, an update to the documentation for testing Swap.zol and also a fix to the Escrow-refs.zol contract.

Changes

  • Previous changes mean that the commitment DB for double testing is no longer shared. This has exposed an issue that the event listener does not pick up commitment preimages encrypted under the shared secrets. This is because the keys used to decrypt are taken at the beginning after the contract is deployed, but for Swap.zol the shared keys are generated later during the getSharedKeys API. This meant that the event listener tries to decrypt within the shared key to decrypt with.
  • In NFTEscrow.zol tokenOwners is a re-initialisable variable. It therefore can be reinitialised even by an entity that doesn't own the variable. We need to make sure that checks are in place so that this only happens in deposit where it is marked as re-initialisable. Previously, in transferFrom tokenOwner[tokenId] is modified, but there is no check that the sender owns it. Normally in Starlight, this would be ok because only the owner could modify it. However, as it is re-initialisable a malicious entity that doesn't own it could still transfer it by re-initialising it. The documentation is also updated to make this clearer.
  • In Swap.zol the tokenOwners mapping is no longer a reinitialisable variable. This is not necessary because it is never set to the zero address.
  • In Escrow-refs.zol logic written just for testing that should not have been included in the final contract has been removed.
  • The fact that the commitment DB for double testing is no longer shared means that the documentation is out of date. We need to call getSharedKeys by both the sender and receiver of the swap and include the public keys of both inside the Swap APIs. Otherwise, both the sender and receiver will not receive their commitment preimages via the event listeneer. Additionally, there was a mistake in the documentation where user A and user B are confused that has been fixed.

Checklist

  • Tests added/updated
  • CI passes
  • No secrets/keys committed
  • Docs updated (if needed)
  • Backwards compatible (or noted breaking change)

How to test

  1. Test NFT_Escrow.zol.
  2. Test Swap.zol.

Screenshots / Evidence (if applicable)

  • Evidence that commitment preimages are now picked up in the event listener:
Screenshot 2026-04-17 at 18 03 16 Screenshot 2026-04-17 at 17 57 43

@lydiagarms lydiagarms changed the title Lyd/swap el bug SwapBugFixes Apr 17, 2026
@lydiagarms lydiagarms requested review from SwatiEY and Wei-257 April 20, 2026 15:41
Comment thread README.md Outdated
Copy link
Copy Markdown

@Wei-257 Wei-257 left a comment

Choose a reason for hiding this comment

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

only one comment on README

@lydiagarms lydiagarms force-pushed the lyd/swapELBug branch 3 times, most recently from 64ac9e7 to 98064ea Compare April 21, 2026 16:11
@lydiagarms lydiagarms merged commit 592cc48 into master Apr 23, 2026
4 checks passed
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 1.10.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants