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

Reconcile addresses with mismatched casings #748

Merged
merged 2 commits into from
May 10, 2024

Conversation

zoeyTM
Copy link
Contributor

@zoeyTM zoeyTM commented May 1, 2024

The problem raised by issue #745 is that currently ignition will fail to reconcile the address param 0xaaa... if it has been changed to 0xAaa... or, more specifically, if the case of the new address doesn't perfectly match the previously executed address.

This makes sense as a bug to fix, but there is a bit of a design decision to be made around what solution we decide on. The solution I opted for, and what is currently in this PR, is the following logic:

  • 0xaaa... + 0xAAA... reconciles ✅
  • 0xaaa... + 0xAaa... does not reconcile ❌

The reason for this choice is because it follows the same rules that ethers uses for their checksum and address-checking code.

An alternative approach would be to cast every address to lowercase before checking equality, which would allow for the second case above to reconcile successfully, but we would be bypassing what (admittedly few) protections are in place for checksummed addresses.

In this PR, I included two tests to demonstrate the above cases, but I'll add them to the rest of the reconciliation tests after we decide on a direction for this logic.


Edit:

Leaving the above for posterity's sake, but we are moving forward with this strategy.

One additional thing this PR does is adds examples/sample to the pnpm-workspace.yaml file for better local testing and debugging of things.

resolves #745

@zoeyTM zoeyTM marked this pull request as ready for review May 3, 2024 06:10
@zoeyTM zoeyTM merged commit f504aeb into development May 10, 2024
5 checks passed
@zoeyTM zoeyTM deleted the address-reconciliation branch May 10, 2024 04:03
@zoeyTM zoeyTM mentioned this pull request May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Handling of case insensitive parameters could be improved
3 participants