Skip to content

[codex] Add wallet signature integration guide#397

Merged
punk6529 merged 3 commits into
mainfrom
codex/wallet-signing-guide
Jun 15, 2026
Merged

[codex] Add wallet signature integration guide#397
punk6529 merged 3 commits into
mainfrom
codex/wallet-signing-guide

Conversation

@punk6529

Copy link
Copy Markdown
Contributor

Summary

Closes #396.

Adds the INT-004 wallet, EIP-712, ERC-1271, and Safe signing guide for app-facing integration work. The new guide gives React, mobile, Electron, operator UI, indexer, and backend signing-service teams a single source for typed-data domain fields, replay/revocation controls, EOA and ERC-1271 validation, Safe and WalletConnect caveats, frontend preflight reads, failure states, no-secret policy, and maintenance triggers.

This also wires the guide into local/CI checks, release-readiness navigation, integration docs, generated release manifest/proof/checksum artifacts, changelog, and the autonomous execution backlog.

Validation

  • python scripts/test_wallet_signature_flows.py
  • python scripts/check_wallet_signature_flows.py
  • make wallet-signature-flows-check
  • python scripts/test_changelog_check.py
  • python -m py_compile scripts\check_wallet_signature_flows.py scripts\test_wallet_signature_flows.py scripts\check_integrations_readme.py scripts\test_integrations_readme.py scripts\check_release_readiness.py scripts\test_release_readiness.py scripts\generate_release_manifest.py scripts\test_release_manifest.py
  • python scripts/test_drop_authorization_fixtures.py
  • python scripts/check_drop_authorization_fixtures.py
  • python scripts/test_signer_custody_readiness.py
  • python scripts/check_signer_custody_readiness.py
  • python scripts/test_drop_authorization_signing_evidence.py
  • python scripts/check_drop_authorization_signing_evidence.py
  • python scripts/test_integrations_readme.py
  • python scripts/check_integrations_readme.py
  • python scripts/test_release_readiness.py
  • python scripts/check_release_readiness.py
  • python scripts/test_release_manifest.py
  • python scripts/check_changelog.py
  • python scripts/generate_risk_register.py
  • python scripts/generate_release_manifest.py
  • python scripts/generate_bytecode_release_proof.py
  • python scripts/generate_release_checksums.py
  • python scripts/test_risk_register.py
  • python scripts/check_risk_register.py
  • python scripts/generate_risk_register.py --check
  • python scripts/generate_release_manifest.py --check
  • python scripts/test_bytecode_release_proof.py
  • python scripts/generate_bytecode_release_proof.py --check
  • python scripts/test_release_checksums.py
  • python scripts/generate_release_checksums.py --check
  • forge test --match-path test/StreamDropsEIP712.t.sol -vvv
  • forge test --match-path test/StreamDropsERC1271.t.sol -vvv
  • python scripts/test_contract_flows.py
  • python scripts/check_contract_flows.py
  • python scripts/test_auction_flows.py
  • python scripts/check_auction_flows.py
  • python scripts/test_windows_check_wrapper.py
  • python scripts/test_windows_ci_wrapper.py
  • bash -n scripts/check.sh
  • PowerShell parser check for scripts/check.ps1
  • git diff --check

Notes

This is documentation and gate wiring only. It does not change Solidity behavior, signing service code, frontend code, CI infrastructure beyond adding the new doc checker to existing gates, or deployment artifacts beyond regenerated release metadata/hash artifacts.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@punk6529, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 2 hours, 51 minutes, and 41 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 16eaa633-527b-4fd6-8987-c51db2e79a41

📥 Commits

Reviewing files that changed from the base of the PR and between 604f0db and 0310488.

📒 Files selected for processing (27)
  • .github/workflows/ci.yml
  • CHANGELOG.md
  • Makefile
  • docs/drop-authorization-signing.md
  • docs/integrations/README.md
  • docs/integrations/wallets-and-signatures.md
  • docs/release-readiness.md
  • docs/signer-custody-readiness.md
  • ops/AUTONOMOUS_RUN.md
  • ops/EXECUTION_BACKLOG.md
  • release-artifacts/README.md
  • release-artifacts/latest/SHA256SUMS
  • release-artifacts/latest/bytecode-release-proof.json
  • release-artifacts/latest/release-checksums.json
  • release-artifacts/latest/release-manifest.json
  • release-artifacts/latest/risk-register.json
  • release-artifacts/signer-custody-readiness/signer-custody-readiness-template.json
  • scripts/check.ps1
  • scripts/check.sh
  • scripts/check_integrations_readme.py
  • scripts/check_release_readiness.py
  • scripts/check_wallet_signature_flows.py
  • scripts/generate_release_manifest.py
  • scripts/test_integrations_readme.py
  • scripts/test_release_manifest.py
  • scripts/test_release_readiness.py
  • scripts/test_wallet_signature_flows.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/wallet-signing-guide

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@6529bot

6529bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

6529bot crypto security analysis - b59b94b

Verdict: No security findings.

This PR adds documentation (docs/integrations/wallets-and-signatures.md), a Markdown checker (scripts/check_wallet_signature_flows.py), its tests, and the corresponding CI/Make/release-manifest wiring plus regenerated checksum/manifest artifacts. There is no production wallet, auth, signature-verification, or token logic in the diff.

I reviewed the doc for security-correctness of its guidance, since wrong advice here can propagate into integrator code:

  • Domain separation, replay/cancellation/epoch controls, and the "EIP-712 is encoding/signing only" caveat are stated correctly (docs/integrations/wallets-and-signatures.md:120-152).
  • The no-on-chain-monotonic-nonce-map clarification and signer-service nonce/salt uniqueness obligation are accurate (docs/integrations/wallets-and-signatures.md:148-156).
  • EOA failure modes (high-s malleability, invalid v, zero recovered signer, EIP-2098 vs 65-byte length) and ERC-1271 fail-closed behavior (32-byte return, 0x1626ba7e magic) are correctly described (docs/integrations/wallets-and-signatures.md:200-235).
  • Payer/value binding (payer == msg.sender when price > 0, zero payer when free, msg.value == price) and chain/domain preflight before signing are sound (docs/integrations/wallets-and-signatures.md, Failure States table).
  • No-secret custody boundaries and the explicit pre-audit / not-production-ready framing avoid overclaiming.

The checker (scripts/check_wallet_signature_flows.py) does safe local-only validation: normalize_repo_path rejects path escapes (line ~120), normalized_link_target skips external/mailto/anchor links so no network fetch occurs, and link resolution is repo-relative with existence checks. No injection, SSRF, or unsafe-fetch surface introduced.

Regenerated release-manifest.json, bytecode-release-proof.json, release-checksums.json, and SHA256SUMS hashes are internally consistent (the manifest sha256 referenced in the bytecode proof and checksums matches 81e0410a…).

No replay, domain-separation, nonce, chain-id, identity-binding, secret-leakage, or injection issues found in changed code.

@6529bot

6529bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

6529bot general PR review - b59b94b

Verdict: Good to merge

This PR is documentation-only: a new wallet/signature integration guide, its checker/test pair, gate wiring, and regenerated release artifacts. I reviewed the checker logic and the manifest/checksum updates for correctness.

Nice-to-have

  • scripts/check_wallet_signature_flows.py:228missing_phrases collapses all whitespace before substring matching, but several required phrases contain backticks (invalid \v`, isValidSignature(bytes32 digest, bytes signature)). Those still match because backticks are preserved in both text and phrase, so this is fine today. However, the phrase name/version/nonce/saltare extremely generic single words that will match incidentally anywhere in the doc (e.g., prose), so they provide near-zero protection against the doc actually dropping the relevant content. Consider tightening these to more specific tokens (e.g., ``name`` table rows orsigner-service-allocated`) so the gate fails meaningfully if the substance is removed.

  • scripts/check_wallet_signature_flows.py:158linked_repo_paths raises on any missing link target, but the REQUIRED_LINK_TARGETS check only verifies presence, not that the link uses a repo-relative resolvable path matching intent. Low risk given the doc is also manifest-tracked, but worth noting the two checks overlap and the required-target list duplicates maintenance burden whenever the doc's link set changes.

No correctness, security, or data-integrity issues found. The regenerated release-manifest.json, bytecode-release-proof.json, release-checksums.json, SHA256SUMS, and risk-register.json hashes are internally consistent (manifest hash referenced by both bytecode proof and checksums match 81e0410a…; risk register and signer-custody runbook hashes propagate consistently). Gate wiring is added uniformly across Makefile, check.sh, check.ps1, and CI, and the doc passes its own asserted heading/phrase/command/link contract.

@6529bot

6529bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

6529bot follow-up commit review - 31e3a47

Verdict: No new findings.

This follow-up commit set matches what the prior same-lane bot reviews (security + general at b59b94bc) already covered: the wallet/signature guide, its checker/test pair, gate wiring, and regenerated artifacts. No substantive new code or content was introduced between the prior reviewed head and this head; the diff is identical in scope.

The two prior nice-to-have observations (generic single-word phrases in REQUIRED_PHRASES at scripts/check_wallet_signature_flows.py:228, and the overlapping link-target checks at scripts/check_wallet_signature_flows.py:158) remain unaddressed but were already raised and are non-blocking, so I won't repeat them.

Manifest/checksum consistency is intact (manifest 81e0410a… propagated through bytecode proof and checksums; risk register and signer-custody runbook hashes consistent). No regressions from the follow-up.

Copy link
Copy Markdown
Contributor Author

Addressed the 6529bot nice-to-have about overly generic checker phrases in 0310488: name, version, chainId, verifyingContract, nonce, salt, and deadline are now required as code-token field names in the wallet/signature checker and its focused tests were updated accordingly.

Validation rerun:

  • python scripts/test_wallet_signature_flows.py
  • python scripts/check_wallet_signature_flows.py
  • python -m py_compile scripts\check_wallet_signature_flows.py scripts\test_wallet_signature_flows.py
  • make wallet-signature-flows-check
  • python scripts/generate_risk_register.py --check
  • python scripts/generate_release_manifest.py --check
  • python scripts/generate_bytecode_release_proof.py --check
  • python scripts/generate_release_checksums.py --check
  • git diff --check

@6529bot

6529bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

6529bot follow-up commit review - 0310488

Verdict: No new findings.

The commit since the prior follow-up marker (31e3a4763def) addresses the earlier nice-to-have about overly generic checker phrases. REQUIRED_PHRASES in scripts/check_wallet_signature_flows.py:55-128 now includes specific code tokens (DROP_AUTHORIZATION_TYPEHASH, DROP_ID_TYPEHASH, consumedDropIds, cancelledDropIds, 0x1626ba7e, EIP-2098, deriveDropId, hashDropAuthorization, etc.) so generic single words like name/version/nonce/salt now sit alongside structural tokens that fail meaningfully if the substance is dropped.

I checked the follow-up for regressions and found none:

  • The new phrase list keeps backtick/parenthesis-bearing phrases (invalid \v`, isValidSignature(bytes32 digest, bytes signature)) matchable since missing_phrases collapses whitespace but preserves those characters; the doc body contains the exact tokens (docs/integrations/wallets-and-signatures.md:200-235`).
  • Manifest/checksum consistency remains intact: manifest 81e0410a… propagates through bytecode-release-proof.json and release-checksums.json, and the risk-register/runbook/changelog hashes are internally consistent across SHA256SUMS, release-manifest.json, and risk-register.json.
  • Gate wiring is unchanged from the already-reviewed scope.

Resolved since last review

  • The prior nice-to-have on generic single-word REQUIRED_PHRASES (scripts/check_wallet_signature_flows.py:228) is addressed: domain field names and signature-critical tokens are now required as specific tokens, and the focused tests were updated accordingly.

The remaining prior observation about overlapping link-target checks (linked_repo_paths vs REQUIRED_LINK_TARGETS) is non-blocking and already raised, so I won't repeat it.

Copy link
Copy Markdown
Contributor Author

Merging after final checks:

  • CI run 27542804209 passed (Foundry smoke, Windows PowerShell wrapper).
  • CodeRabbit status context is success; the rate-limit summary is informational/non-blocking and no actionable CodeRabbit threads are open.
  • 6529bot security review reported no findings.
  • 6529bot general review said good to merge; its generic-phrase nice-to-have was addressed in 0310488.
  • 6529bot latest follow-up on 0310488 reports no new findings and confirms that nice-to-have is resolved.
  • Review threads are empty.

@punk6529 punk6529 merged commit 2dba644 into main Jun 15, 2026
3 checks passed
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.

INT-004: Add wallet, EIP-712, ERC-1271, and Safe signing guide

1 participant