ENSIP-15 support #3000
ENSIP-15 support #3000
Conversation
|
This is ready for a first review. The way I implemented it follows pretty true to the spec descriptions through each section. This isn't necessarily the fastest way to implement it but it may be easier to read this way. I don't see it presenting much of an issue but if we want to optimize it, I think we can tackle that in a separate PR. |
|
We should also think about documenting this but I'm thinking also in a separate PR? |
The goal for ENS is for ENSIP-15 to entirely replace ENSIP-1 for normalization so I think on our end it would make sense to remove the flag in |
- This more accurately catches cases where more than one blank space may be passed in, which still constitutes an empty name.
- Appropriately check for empty name, not just ``None`` value
- Begin implementing ENSIP-15 - Add spec json files directly from the specification and add the tests json file - Begin writing the normalization test(s) under tests/ens/normalization
…any`` - Asked a question on the original PR for ENSIP-15 and got a good response back. This should be ``all()`` and not ``any()``. - Added a note for clarification
- Some cleanup, mostly linting.
- Add back the logic that exists for ``normalize_name`` and add a flag for normalizing via ENSIP-15 so as not to break the library. This flag will likely be removed and become the default in web3.py ``v7``. - Add a flag on the ENS class itself that turns on ENSIP-15 normalization for all internal class methods - Re-structure some imports and get rid of circular dependency issue
- addresses some comments on PR ethereum#3000
- also addresses comments on PR ethereum#3000
- also addresses comments on PR ethereum#3000
- also addresses comments on PR ethereum#3000
There was a problem hiding this comment.
This ENSIP was a doozy! Thanks for taking it on! Overall this looks really great. Just a few observations:
- Can we catch the warnings that are emitted in the existing ens tests? :)
- It might be more understandable to split some of the big functions (in
ens/_normalization.pyin particular) into smaller functions with names that describe what they do, even though they don't get re-used. Feel free to take or leave this one - It might also be nice to do some smoke testing to make sure the flag is working on the static methods. For example, pass an ens name where
nameprep('...', ensip15=True)return a different result thannameprep('...', ensip15=False)
Which tests?
Coming up in a separate PR with documentation on this as well 👍🏼
These were just helped methods to help me extract information from the json files. I can do a pass-through and try to clarify some more things 👍🏼 edit: Oh I see which tests. All of them basically 😆 ... yeah that's a good point. I'll see what I can do 🤔 |
- also addresses comments on PR #3000
What was wrong?
Closes #2967
How was it fixed?
See specification here: ENSIP-15
Todo:
Cute Animal Picture