Sync v3 session encoding with wallet-contracts-v3#365
Conversation
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>
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>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca80af7e18
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05ac5ffbcf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case float64: | ||
| issuedAt = uint64(v) |
There was a problem hiding this comment.
When AttestationFromJson receives authData.issuedAt as a JSON number rather than the string form, this direct cast accepts invalid values instead of returning an error: for example -1 becomes math.MaxUint64, and fractional/out-of-range numbers are truncated or implementation-dependent before being appended to the attestation bytes. This can make the library hash and sign a different attestation than the JSON input represents, so the numeric path should reject negatives, fractions, and values outside uint64 just like the string path does.
Useful? React with 👍 / 👎.
|
LGTM just that small comment |
2 participants
go-sequence's session encoding was ported before several contract changes and never caught up, so any explicit-session config, attestation, or session-call digest it produced was rejected on-chain. This re-syncs it (cross-checked against the sequence.js primitives):
chainId(32 bytes aftersigner) and encodedeadlineasuint64(8 bytes, was 32) — wallet-contracts-v3cc6890eand8a376ad. MatchesSessionSig.recoverConfiguration/permission.ts.issuedAt(uint64, 8 bytes) toauthDataso the attestation hash matchesLibAttestation.toHash—12b8c27. Serialized as a decimal string in JSON to matchattestation.ts.msg.sender(the verifying contract), not a parent entry, so the signing context's trailing parent must be removed — the wallet-bound digest comes from the partial-payload-replay audit fix (9c19609, Code4rena S-93/S-169, wallet-contracts-v3#89, the same fix Implement session call digest for v3 sessions #362 implements).issuedAtthrough thecmd/sequenceCLI input.Encoding vectors in the added tests are verified against the contracts via forge (
recoverConfiguration,LibAttestation.toHash,hashPayloadCallIdx). Only the current (Rc4+) session manager is targeted; the deprecated Dev1/Dev2/Rc3 digest schemes are intentionally not supported.🤖 Generated with Claude Code