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.

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

fix: lowercase getNonce address #350

Merged
merged 1 commit into from
Feb 2, 2024
Merged

Conversation

marktoda
Copy link
Contributor

@marktoda marktoda commented Feb 1, 2024

Orders are sanitized to have lowercased addresses before pumping into
the DB. This means all offerers in the nonce db also have lowercased
addresses. However, the getNonce query does not do sanitization, so if
any integrator requests eip-55 cased nonces, it will not find results in
the db. This commit sanitizes

However, we should hold out for the frontend to make a fix incrementing
the nonce. It's currently relying on nonce randomness to get new nonces

@marktoda
Copy link
Contributor Author

marktoda commented Feb 1, 2024

cc @tinaszheng @just-toby

Orders are sanitized to have lowercased addresses before pumping into
the DB. This means all `offerers` in the nonce db also have lowercased
addresses. However, the getNonce query does not do sanitization, so if
any integrator requests eip-55 cased nonces, it will not find results in
the db. This commit sanitizes

However, we should hold out for the frontend to make a fix incrementing
the nonce. It's currently relying on nonce randomness to get new nonces
@marktoda marktoda force-pushed the fix-getNonce-query-address-casing branch from 54f03fb to 1d9994a Compare February 1, 2024 23:22
Copy link
Member

@ConjunctiveNormalForm ConjunctiveNormalForm left a comment

Choose a reason for hiding this comment

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

o m g

@marktoda marktoda merged commit 1c07d2f into main Feb 2, 2024
3 checks passed
@marktoda marktoda deleted the fix-getNonce-query-address-casing branch February 2, 2024 18:45
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