Skip to content

Implement session call digest for v3 sessions#362

Merged
taylanpince merged 2 commits into
masterfrom
v3-session-call-digest
Jul 13, 2026
Merged

Implement session call digest for v3 sessions#362
taylanpince merged 2 commits into
masterfrom
v3-session-call-digest

Conversation

@Agusx1211

@Agusx1211 Agusx1211 commented Jul 10, 2026

Copy link
Copy Markdown
Member

This implements the digest that a session key has to sign for each call of a v3 payload, needed for waas smart-sessions (7193). The old hashCallWithReplayProtection was a leftover stub, unexported and with no callers, so I replaced it with an exported HashCallWithReplayProtection that matches SessionSig.hashPayloadCallIdx in wallet-contracts-v3: keccak256(Payload.hashFor(payload, wallet) ++ uint256(callIdx)).

Note that the digest scheme the stub hinted at (chainId ++ space ++ nonce ++ callHash) is no longer what the contracts verify, it was removed in the audit fix for partial payload replay (0xsequence/wallet-contracts-v3#89), so implementing it as described would produce signatures that never pass on-chain.

The test vectors in session_digest_test.go were generated from SessionSig.hashPayloadCallIdx itself, with a forge test constructing identical payloads, so the test asserts parity with the on-chain verification. I also checked that a signature produced over this digest recovers to the right signer through plain ecrecover, which is the exact path the contract uses (no EIP-191 prefix).

@ScreamingHawk ScreamingHawk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The old hashCallWithReplayProtection was replaced with hashPayloadCallIdx in the contract as per the PR text. I suggest renaming this function to match the new name too.

Otherwise LGTM!

Comment thread core/v3/session.go Outdated
@taylanpince
taylanpince merged commit a201553 into master Jul 13, 2026
6 checks passed
@taylanpince
taylanpince deleted the v3-session-call-digest branch July 13, 2026 13:59
ScreamingHawk added a commit that referenced this pull request Jul 13, 2026
#362 was merged with the digest function renamed to HashPayloadCallIdx;
update the parent-wallets test to the new name so the package builds.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ScreamingHawk added a commit that referenced this pull request Jul 19, 2026
* fix(v3): sync session encoding with wallet-contracts-v3

The explicit-session and implicit-attestation encodings had drifted from
the deployed v3 contracts, so any config image hash / session signature
go-sequence produced was rejected on-chain. Re-sync three changes the
contracts made (and mirror the sequence.js primitives JSON):

- SessionPermissions: add chainId (32 bytes, after signer) and encode
  deadline as uint64 (8 bytes) instead of 32. Matches SessionSig
  recoverConfiguration and permission.ts.
- Attestation authData: append issuedAt (uint64, 8 bytes) so the
  attestation hash matches LibAttestation.toHash. Serialize issuedAt as a
  decimal string in JSON to match attestation.ts.
- Wire issuedAt through the cmd/sequence CLI input.

Add regression tests with byte vectors verified against the contracts via
forge (recoverConfiguration / LibAttestation.toHash).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(v3): drop session wallet from call digest parent wallets

HashCallWithReplayProtection hashed the payload's parentWallets verbatim.
The session-holding wallet is the last parent wallet in the signing
context, but on chain it is the verifying contract (msg.sender), not a
parent entry, so it must be dropped before hashing to match
SessionSig.hashPayloadCallIdx. Wallet-bound digest comes from the partial
payload replay audit fix (wallet-contracts-v3 9c19609, Code4rena
S-93/S-169, #89).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(v3): validate session deadline fits in uint64

EncodeSessionPermissions encodes deadline into an 8-byte buffer via
big.Int.FillBytes, which panics on a negative value or one that does not
fit in uint64 (e.g. a max-value "no expiry" deadline). Validate the range
and return an error instead of crashing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(v3): validate numeric issuedAt in attestation JSON

AttestationFromJson accepted a JSON-number issuedAt via an unchecked
uint64 cast, so a negative, fractional or out-of-range value was silently
coerced (e.g. -1 -> MaxUint64) and the library would hash/sign a
different attestation than the JSON represented. Reject negatives,
fractions and values outside uint64, matching the string path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(v3): align parent-wallets test with HashPayloadCallIdx rename

#362 was merged with the digest function renamed to HashPayloadCallIdx;
update the parent-wallets test to the new name so the package builds.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(v3): decode session/attestation numbers safely, validate ranges

Numeric JSON attributes were decoded and encoded unsafely: an unquoted
number above 2^53 was silently rounded by encoding/json before use, and a
negative or over-wide chainId/valueLimit was encoded as its absolute value
or spilled past the fixed 32-byte field.

Add bigIntFromJSON, a single decoder shared by chainId, valueLimit,
deadline and attestation issuedAt: it parses strings and json.Number
exactly and rejects float64 that is non-integer or at/above 2^53.
EncodeSessionPermissions now rejects chainId and valueLimit that are
negative or wider than uint256.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

3 participants