Conversation
|
Thanks for this! I'm off at the moment, but will review when I'm back. Feel free to tag @0kage-eth as well. |
A Safe below 1.3.0 uses a chainId-less EIP-712 domain and one below 1.0.0 a different SafeTx gas field, so hashing under a silently assumed 1.4.1 produces a confidently wrong hash with no signal. Print an unmissable stderr warning whenever the default is used. Hash output is byte-identical and the warning goes to stderr, so --json consumers are unaffected.
An unflagged delegatecall is the highest-risk action a Safe signer can approve: the target's code runs with the Safe's own storage and identity. safe_hashes.sh warns on operation=1; clearsig printed hashes only. Now safe-hash flags every --operation 1 on stderr and checks the target against the official MultiSend deployments from safe-global/safe-deployments: a known batching contract is named, an unknown target gets the strongest warning. Hash output is unchanged and --json stdout stays byte-identical.
6e13540 to
9aab8a8
Compare
|
Thanks @PatrickAlphaC — no rush. Rebased onto current The rebase was clean on the code: Ran the full CI sequence locally against the new base: One thing you may need to do: the CI run on this PR is sitting at |
While validating
clearsig safe-hash0.3.1 against live on-chaingetTransactionHashvalues (Sepolia and Fuji, single-call and MultiSend batchedvectors), the hash math checked out byte-for-byte in every case. Two things a
signer-facing tool should say out loud, however, stayed silent. This PR adds both
warnings without changing a single byte of hash output.
1.
--safe-versionis assumed silentlysafe-hashandsafe-msgdefault--safe-versionto1.4.1with no signal. ASafe below 1.3.0 uses a chainId-less EIP-712 domain, and one below 1.0.0 a
different SafeTx gas field, so hashing an older Safe under the assumed default
produces a confidently wrong hash. For a tool whose whole job is catching wrong
hashes, a silent version assumption is the worst-case failure mode.
The first commit keeps the default but prints an unmissable stderr warning
whenever it is used:
If you would rather make
--safe-versionrequired (a breaking CLI change, butthe strictest fix), I am happy to rework the commit that way; the tests cover
both behaviors easily.
2.
--operation 1prints hashes with no delegatecall warningAn unflagged delegatecall is the highest-risk action a Safe signer can approve:
the target's code runs with the Safe's own storage, balance, and identity (the
Bybit incident vector).
safe_hashes.shwarns on delegatecall; clearsig printedthe three hashes only.
The second commit flags every
--operation 1on stderr and checks the targetagainst the official MultiSend deployments from
safe-global/safe-deployments
(1.1.1 through 1.5.0, canonical/eip155/zksync):
and for anything that is not a known MultiSend:
Output compatibility
All warnings go to stderr. stdout (both the human table and
--json) isbyte-identical to 0.3.1; scripts parsing the output are unaffected.
Verification
uv run ruff check,uv run ruff format --check,uv run ty check, anduv run pytestall pass; 10 new CLI tests cover warning presence, absence,case-insensitive target matching, and the
--jsonpath staying clean.getTransactionHashview calls on Sepolia 11155111 and Fuji 43113, single-calland hand-packed 2-call MultiSend vectors, operation sensitivity, bit-flip
negative, domain-hash cross-check) against this branch: 12/12 pass, every hash
identical to 0.3.1.
Before/after transcript on a real Sepolia vector (copy-paste reproducible)
The vector is a live Safe 1.4.1 on Sepolia (
0xf55FEba2D9c767Bf8018362d9eE4F378dcEB727E,nonce 23); the expected safeTxHash
0xce4568d5...is the value the Safe itself returns froman on-chain
getTransactionHashview call.0.3.1 (no
--safe-versiongiven, nothing said about it):This branch (same command, warning on stderr, hashes byte-identical):
Same for the delegatecall leg:
--operation 1 --to 0x9641d764fc13c8B624c04430C7356C1C7C8102e2(MultiSendCallOnly 1.4.1) prints hashes only on 0.3.1 and the named warning on this branch,
with identical hashes.