refactor: dedupe create/lock/init helpers, simplify deployment-code build#36
Merged
Conversation
…uild
- Add _prepare() shared by createAccount and computeAddress so the
effective salt, deployment code, and address are built and hashed once
instead of twice (avoids constructing/keccak-ing the ~24KB code buffer
and the actors commitment a second time per creation).
- Rebuild _buildDeploymentCode with a single abi.encodePacked of the
14-byte loader + bytecode, dropping the byte-by-byte copy loop.
- Add _isInitialized() and use it in createAccount/importAccount,
removing the triplicated localSequence/multichainSequence check and
restated "localSequence doubles as the initialized flag" comment.
- Rewrite _checkAndClearLock in terms of _isLocked so the lazy-clear rule
("LOCKED still set but no longer locked") is self-evident.
- Fix isActor NatSpec: it reports authorization, not liveness (no expiry
check); standardize getActorConfig on the >= K1_AUTHENTICATOR idiom.
…henticator - Mark ERC1271_SELECTOR `internal constant` (was the only constant with no explicit visibility), matching SECP256K1_HALF_ORDER. - Document why _authorizeActor accepts a non-zero authenticator with no code: authenticators may be counterfactual or intentionally codeless sentinels, so it fails closed at authentication time by design.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
_prepareDeployment()shared bycreateAccount+computeAddress.createAccountpreviously derived the address viacomputeAddress, then rebuilt the deployment code and effective salt from scratch — constructing and keccak-ing a ~24KB buffer (and recomputing the actors commitment) twice per creation. Now done once for both callers._buildDeploymentCodeviaabi.encodePacked. Replaces 14 indexed byte-stores + a trailing bounded copy loop with a single packed encode of the loader + bytecode (same loader bytes, compiles to a calldatacopy, easier to audit).isActorNatSpec fix + idiom standardization.isActorreports authorization, not liveness (it does not check expiry — intentional,_revokeActorrelies on it); doc corrected to "authorized (possibly expired)".getActorConfigstandardized on the>= K1_AUTHENTICATORnamespace idiom._checkAndClearLockin terms of_isLocked. Removes the duplicated three-branch flag decode; the lazy-clear rule ("FLAG_LOCKEDstill set but no longer locked") is now self-evident._isInitialized()helper. Centralizes the triplicatedlocalSequence || multichainSequenceinitialized check acrosscreateAccount/importAccount.ERC1271_SELECTORmarkedinternal constant(was the only constant without explicit visibility); documented why_authorizeActoraccepts a non-zero-but-codeless authenticator (counterfactual/sentinel authenticators, fails closed at auth time).Test plan
forge buildclean on 0.8.36forge test— 355 passed, 0 failed