ERC-7529 Refactor #1056
Draft
ERC-7529 Refactor #1056
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's a really expensive, nonsensical, and unused method. Refactored to use ERC7529Utils. Removed DNSRepository (ERC7529Utils does this now). Removed unused ConsentContractRepositoryError
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Release Notes
JIRA Link
Summary:
This refactors the core to use our ERC-7529 package. It removes the DNS repository and methods from the consent contract repository, in favor of our package. It update the ERC-7529 package to be backwards-compatible with the existing snickerdoodle-protocol TXT records as well as the new ERC-7529 ones. Along the way, I found a really crazy method that I removed, as it wasn't used anywhere, was a total pain to refactor, and was really expensive and inefficient if it ever was called.
Intended results:
This should be completely transparent. DNS-based public invitations should continue to work, with both the TXT record at snickerdoodle-protocol.{domain} AND the new ERC-7529 compliant erc-7529.${chainId}._domaincontracts.{domain}.
Potential Failures:
Existing DNS based public invitations could break. The method I removed could be used somewhere that's not apparent and not caught by the compiler, though I doubt it. Contracts that include more than the ETLD+1 (phoebe.com), such as a subdomain (child.phoebe.com) or a path (phoebe.com/child), will almost certainly break- and this is to be expected. We'll have to update the IP so that when you are entering domains on an audience you can only enter the ETLD+1.
Testing Notes:
ERC-7529 was unit and integration tested. Unit tests for invitation service (that was refactored) were updated.
Pre-Flight Checks