Skip to content

enforce address normalization at compile-time #811

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

attente
Copy link
Contributor

@attente attente commented Jul 14, 2025

No description provided.

@attente attente force-pushed the new-address-type branch 4 times, most recently from e2c3167 to d3bb7b6 Compare July 15, 2025 18:12
@corbanbrook
Copy link
Contributor

Although i do think this is an interesting approach with some obvious pros i have a few concerns.

  1. Address is ambiguous with Address type from ox or viem. Auto import may pull the wrong type and this will allow errors to slip through as types are compatible by only ours enforces normalized.

We should rename our type to something like NormalizedAddress to create a clearer separation.

  1. I believe this type should only be used internally and not on public methods as it would force the users of sequence to know about an obscure implementation detail. They would need to normalize with our tool for any addresses passed in, and sometimes these addresses would be within other datatypes that they would need to process beforehand.

  2. Perhaps we would want a custom lint rule to enforce that nothing was cast to NormalizedAddress to ensure that adresses were in fact normalized as casting would also inject a loop hole into the system that could allow errors.

Would love to hear what other people think of this.

@attente attente force-pushed the new-address-type branch from d3bb7b6 to 860c4d8 Compare July 26, 2025 01:39
@attente attente marked this pull request as ready for review July 26, 2025 01:45
@attente attente requested review from a team as code owners July 26, 2025 01: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