Skip to content

Conversation

@jvaleskadevs
Copy link
Contributor

Description

Fix trade coin invalid nonce when signing typed data. Owner and Token parameters were mixed.

Motivation and Context

  • The allowance mapping definition is:
    mapping (address owner => mapping (address token => mapping (address spender => PackedAllowance))
  • But the previous code was setting the token address first in the args of the allowance call to fetch the nonce.
  • This may work when the nonce is Zero in the first call but will break later.
Screenshot from 2025-09-05 20-03-06 Screenshot from 2025-09-05 20-04-31
  • Example of previous state.. always returning nonce equals Zero.
Screenshot from 2025-09-05 20-00-59
  • Example after fixing it. Returning the right nonce.

Does this change the ABI/API?

  • This changes the ABI/API

What tests did you add/modify to account for these changes

  • No tests.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New module / feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I added a changeset to account for this change

Reviewer Checklist:

  • My review includes a symposis of the changes and potential issues
  • The code style is enforced
  • There are no risky / concerning changes / additions to the PR

@iainnash
Copy link
Collaborator

iainnash commented Sep 5, 2025

Good catch thank you!

@iainnash iainnash self-requested a review September 5, 2025 19:43
@iainnash iainnash merged commit b911cb0 into ourzora:main Sep 5, 2025
2 checks passed
@jvaleskadevs jvaleskadevs deleted the fix-tradecoin-invalid-nonce branch September 5, 2025 20:08
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.

2 participants