Skip to content

feat(MultiFactor): remove registry dependency#33

Open
zeroknots wants to merge 2 commits into
mainfrom
feat/remove-registry-multifactor
Open

feat(MultiFactor): remove registry dependency#33
zeroknots wants to merge 2 commits into
mainfrom
feat/remove-registry-multifactor

Conversation

@zeroknots
Copy link
Copy Markdown
Member

Summary

Removes the ERC-7484 registry dependency from MultiFactor. Drops ERC7484RegistryAdapter inheritance, the IERC7484 constructor argument, and the REGISTRY.checkForAccount attestation checks in onInstall and setValidator. The module can now be deployed and used without a registry.

Test plan

  • Update existing MultiFactor test constructors to drop the registry argument
  • Run forge test against the MultiFactor suites

Drops ERC7484RegistryAdapter inheritance and the subvalidator attestation
checks in onInstall/setValidator so the module no longer requires a registry
at construction or use.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown

@rhinestone-kevin rhinestone-kevin Bot left a comment

Choose a reason for hiding this comment

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

Trivial change — approved without a full review.

Adds onlyBuiltDependencies entries so pnpm install succeeds without
manual approval for git-hosted Rhinestone packages.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown

@rhinestone-kevin rhinestone-kevin Bot left a comment

Choose a reason for hiding this comment

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

Request changes — removes the MultiFactor registry dependency, but the MultiFactor tests still use the old constructor.

uint256 iteration => mapping(address subValidator => IterativeSubvalidatorRecord record)
) internal iterationToSubValidator;

constructor(IERC7484 _registry) ERC7484RegistryAdapter(_registry) { }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

blocker — Removing the only constructor makes MultiFactor a no-arg contract, but test/MultiFactor/unit/concrete/MultiFactor.t.sol:46 and test/MultiFactor/integration/concrete/MultiFactor.t.sol:44 still instantiate new MultiFactor(...) with a registry argument. That leaves the PR in a non-compiling state once the MultiFactor suites are built.

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.

1 participant