Rename MoiraiDelegate → TransferSettingsPolicy, scope to token transfers only#31
Open
Rename MoiraiDelegate → TransferSettingsPolicy, scope to token transfers only#31
Conversation
…fers only
Audit feedback: MoiraiDelegate allowed arbitrary (target, value, callData) which
in the worst case enables full SCW takeover. Scope it down to native ETH and ERC20
transfers only — the contract now constructs transfer calldata internally.
Key changes:
- Replace MoiraiConfig {target, value, callData} with TransferConfig {recipient,
amount, tokenContract} — tokenContract=address(0) for ETH, ERC20 addr otherwise
- Add ZeroRecipient/ZeroAmount validation at install time
- Add UnexpectedActionData guard (actionData must be empty)
- Add _onPostExecute balance check to catch ERC20 tokens that return false
without reverting (e.g. USDT-mainnet) — reverts with ERC20TransferFailed
- Add explicit Unauthorized guard for delay-only configs in executor uninstall path
- Use abi.encodeCall throughout for compile-time type safety
- EIP-712 domain: "Transfer Settings Policy" (was "Moirai Delegate")
- Rename deploy script to DeployTransferSettingsPolicy.s.sol
Tests (331 passing):
- New: ZeroRecipient, ZeroAmount revert cases
- New: ERC20 transfer happy path (MintableERC20 mock)
- New: FalseReturningERC20 mock + test for silent-false ERC20 guard
- New: UnexpectedActionData revert
- New: pre-install disable path tests (success, delay-only rejection, expired deadline)
- Replace executesContractCall with executesERC20Transfer
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
Addresses audit feedback from Alexis Williams:
MoiraiDelegateallowed arbitrary(target, value, callData)execution which in the worst case enables full SCW takeover (e.g. callingremoveOwner). Since the only real use-case is ERC20/ETH transfers, scope it down — the contract now hard-codes thetransferselector and constructs calldata internally.MoiraiConfig { target, value, callData }withTransferConfig { recipient, amount, tokenContract }—tokenContract = address(0)for native ETH, ERC20 address otherwiseZeroRecipient/ZeroAmountvalidation at install timeUnexpectedActionDataguard — executoractionDatamust be empty0x_onPostExecutebalance check to catch ERC20 tokens that returnfalsewithout reverting (e.g. USDT-mainnet) → reverts withERC20TransferFailedUnauthorizedguard for delay-only configs in the executor uninstall pathabi.encodeCallthroughout for compile-time type safety"Transfer Settings Policy"(was"Moirai Delegate")DeployTransferSettingsPolicy.s.solTest plan
forge test -vv— 331 tests, 0 failuresZeroRecipient,ZeroAmount,NoConditionSpecifiedFalseReturningERC20mock +test_reverts_whenERC20TransferReturnsFalse— verifies_onPostExecutebalance checktest_reverts_withNonEmptyActionData—UnexpectedActionDataguard0x999E14Ac9261F4d32Fe964666305aEC67D90215b, estimated gas 3,716,827 (~0.000041 ETH)🤖 Generated with Claude Code