fix(llm): simulate Safe multisend batches, read Vyper before-state, describe control transfers - #396
Merged
Merged
Conversation
…escribe control transfers A yChad multisend (Funding Distributor set_management + three yETH recovery payouts) was reported with "simulation skipped", "prior claimable amounts were not provided", and the management change described only as a pending handover. All three gaps were fixable from on-chain data. - Simulate multisend batches whose inner transactions are all CALLs as one ordered bundle from the Safe. MultiSend makes each such call from the Safe, so the bundle is the real execution. Only batches with an inner DELEGATECALL or an unknown delegate target still skip simulation. - Read before-state for Vyper setters: find writes through `self.`, parse `public(...)` / `public(HashMap[K, V])` declarations, and expand array key arguments (`set_claimable(address[], ...)`) into one getter read per element (capped at 12, overflow marked unavailable). Solidity batch setters benefit from the array expansion too. - Add a control-transfer adapter (any protocol). For set_management, transferOwnership, setGovernance, setAdmin, role-manager setters and grantRole it reports the current and proposed holder: EOA, EIP-7702 account, Safe (m-of-n), or contract followed one hop to its own controller, plus any operator whitelist, whether the transfer is two-step, and the change in signing threshold (6-of-9 -> 2-of-4 here). - Label the source-context state variables as the ones the function writes. The model had claimed set_claimable leaves `unclaimed` stale. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nding slot Review of #396: the control-transfer adapter called any transfer two-step when the target exposed a pending getter. BoringOwnable's transferOwnership(owner, direct, renounce) has pendingOwner() yet transfers immediately with direct=true, so the report would say the call "only nominates" when control actually moves at once. Two-step is now decided from the call: - nominate-only setters (setPendingOwner, ...) are two-step; - BoringOwnable follows its `direct` flag; - otherwise the setter's source decides: writing only the pending slot (private `_pendingOwner` included) means two-step, writing only the role slot means immediate; - a pending slot the source doesn't resolve is reported as undetermined, never asserted; no pending slot means immediate. find_state_var_writes gains include_private for the Ownable2Step case, and on_chain_state's source resolver is now public for reuse. Co-Authored-By: Claude Opus 5.5 (1M context) <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
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.
Problem
The yChad (Yearn 6-of-9) Safe queued nonce 2296: a MultiSendCallOnly batch that nominates Executor as Funding Distributor management, then pays three late yETH-recovery claimants from yChad's own vault shares. The AI report got the mechanics right but missed the facts that matter:
operation == 0inner tx a plain CALL from the Safe, so a bundle from the Safe is the real execution.recovery_rate.mapping(...) public), and never read array-keyed setters likeset_claimable(address[], uint256[]).Changes
protocols/safe/multisend.py,protocols/safe/main.py:is_simulatable_multisend(): the outer call is a DELEGATECALL into a canonical MultiSend utility and every inner op is a CALL.extract_inner_callsnow carries each inneroperation.utils/source_context.py:self.<name>(with index/member chains, augmented assignments, and one level of nested index).set_claimableleavesunclaimedstale.utils/on_chain_state.py:public(T)/public(HashMap[K, V])parsing; non-publicstorage is skipped.set_claimable(address[], …)readsclaimable(account)per element, capped atMAX_ARRAY_KEY_READS= 12, with overflow marked unavailable. Solidity batch setters benefit too.utils/llm/control_transfer_context.py(new adapter, any protocol):set_management/transferOwnership/setPendingOwner/setGovernance/setAdmin/ role-manager setters andgrantRole(bytes32,address).transferOwnership(owner, direct, renounce)followsdirect. Otherwise the setter's source decides: writing only the pending slot (including private_pendingOwner) means two-step, writing only the role slot means immediate. An unresolved case is reported as "could not be determined".utils/llm/README.md.Result on the same batch (local end-to-end run)
The call flow now shows
Batch simulation: SUCCESSfor all seven calls. The gist gains a Current State section (claimable per account,unclaimed,pending_management) and a Protocol Context section describing both controllers.Not addressed
TestSafeApiQuotatests fail locally. They blankSAFE_API_KEY/SAFE_API_KEY_2but notSAFE_API_KEY_3, so a real key in.envleaks in. With the extra keys blanked, all 38 Safe tests pass. This predates this PR and is unrelated to it.Testing
uv run pytest tests/: 1572 passed. The 4 failures are the environment-dependent quota tests above._explain_safe_txrouting.self.write detection, nested index.ruff formatandruff checkare clean. mypy is clean on the new module.explain_batch_transactionrun.🤖 Generated with Claude Code