Skip to content

Retire legacy ethers signing patch and verify retained SDK safety contracts - #53

Merged
SYMBaiEX merged 3 commits into
mainfrom
sym/upstream-patch-retirement
Oct 26, 2026
Merged

SYMBaiEX merged 3 commits into
mainfrom
sym/upstream-patch-retirement

Conversation

@SYMBaiEX

@SYMBaiEX SYMBaiEX commented Oct 26, 2026 •

Copy link
Copy Markdown
Owner

Retires the ethers signing-key patch by removing its only incoming legacy dependency chain. The hash package imports the signer dependency only in its domain/field declarations; a scoped alias supplies the current official ethers package. Its runtime hashing code remains unmodified, and the wallet plugin continues using ethers 6 for signing.

  • Remove the signing patch, elliptic exclusion and signing package extension; the legacy abstract provider, signing-key, transactions, network and web packages disappear.
  • Guard both CJS/ESM hash runtime files against importing the type-only alias, compile hash declarations with library checking enabled, compare typed-data hashes and ABI calldata with ethers 6, and exercise signing/recovery through the real wallet plugin's dependency.
  • Add SDK atomic-publication regressions for same-clock writes, exclusive ownership and predictable symlinked temporary names. An isolated probe of the integrity-verified unmodified published helper reproduces all three failures; the retained patch passes.
  • Record the fresh registry/source inspection and the decision to stay with published SDK releases while completing the current product. Six patches and three compatibility workspaces remain; no unsupported SDK source build, renamed shim, tool removal or new product feature is introduced.

The lockfile regenerates peer contexts and reconciles existing peer selections. SDK package selections are unchanged.

Local validation: 194 boundary tests; all workspace/tooling types, lint, acceptance checks and zero-production-advisory policy passed. Seven source-runtime tests and four native desktop journeys passed. Real linked-Codex coding passed through ElizaOS in CLI (33.4s) and desktop (49.9s), with source repair independently verified. Wallet action imports, Uniswap calldata, signing and typed-data recovery passed offline; no transactions were submitted. Final PR CI passed, including CodeQL, clean-HEAD packaged acceptance and native Windows provider lifecycle behavior.

Summary by CodeRabbit

  • Bug Fixes
    • Added regression coverage for the retained SDK atomic-publication safety patch; its runtime behavior is unchanged.
  • Maintenance
    • Updated SDK dependency compatibility; legacy signing-related packages are no longer included in the installed dependency graph.
  • Documentation
    • Updated compatibility notes to reflect the current patch and workspace counts, and clarified remaining upstream compatibility requirements.

Copilot AI balanced review requested due to automatic review settings October 26, 2026 06:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 43725d6e-03f0-4f20-a484-6daee3dda910
Commits

Reviewing files that changed from the base of the PR and between 5bed7b9 and 77c154e.

Files ignored due to path filters: 1
  • nub.lock is excluded by !**/*.lock
Files selected for processing: 7
  • docs/engineering/review-findings-follow-up-2026-10-26.md
  • docs/engineering/sdk-package-upgrades.md
  • package.json
  • patches/@ethersproject+signing-key@5.8.0.patch
  • tests/boundary/legacy-dependency-replacements.test.ts
  • tests/boundary/sdk-atomic-publication.test.ts
  • tests/boundary/wallet-runtime-boundary.test.ts
Files with no reviewable changes: 1

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The PR removes the ethers signing-key patch and updates the dependency override to use ethers 6.17.0 for a type-only dependency. It adds boundary tests for dependency compatibility and atomic JSON publication, and updates the engineering documentation to report six remaining patches.

Changes: SDK Patch Retirement

Layer / File(s) Summary
Replace the signing-key dependency
package.json, patches/@ethersproject+signing-key@5.8.0.patch, tests/boundary/legacy-dependency-replacements.test.ts, tests/boundary/wallet-runtime-boundary.test.ts
The package override supplies ethers 6.17.0 for the hash package’s type-only dependency. The signing-key patch and its package entry are removed. Boundary tests check dependency imports, declarations, encoding, signing, and recovery.
Update the compatibility record
docs/engineering/sdk-package-upgrades.md, docs/engineering/review-findings-follow-up-2026-10-26.md
The docs record the patch retirement and reduced count, and describe remaining SDK compatibility requirements and review findings.

Atomic Publication Boundary Tests

Layer / File(s) Summary
Check atomic publication boundaries
tests/boundary/sdk-atomic-publication.test.ts
New tests cover concurrent asynchronous writes, exclusive writes, and symlink targets at the former predictable temporary filename.

Priority: Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Other

Possibly related PRs

  • SYMBaiEX/doolittle#49: Updates the same SDK compatibility inventory and records the patch retirement from seven to six.
  • SYMBaiEX/doolittle#47: Continues the SDK compatibility patch retirement and updates the package graph.
  • SYMBaiEX/doolittle#52: Adds the reliability follow-up that this PR updates with further regression evidence.
Merge Risk: Minimal · up to 77c15

The new boundary tests can run through the supported workflows, and no actionable issue remains before merge.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (3 skipped: 3 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check Warning The description provides a detailed outcome, scope, compatibility rationale, verification results, and CI status. It does not use the required template headings, list exact validation commands and res… Rewrite the description using the required Outcome, Scope, Verification, and Risk and rollback sections. Include the ElizaOS primitive or compatibility boundary, exact commands with results, each applicable verification checkbox, and explic…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly describes the primary change: retiring the legacy ethers signing patch and verifying retained SDK safety contracts.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Full details: Description check

Explanation

The description provides a detailed outcome, scope, compatibility rationale, verification results, and CI status. It does not use the required template headings, list exact validation commands and results, or describe compatibility, migration, security, release, and rollback considerations.

Resolution

Rewrite the description using the required Outcome, Scope, Verification, and Risk and rollback sections. Include the ElizaOS primitive or compatibility boundary, exact commands with results, each applicable verification checkbox, and explicit compatibility, migration, security, release, and rollback considerations.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@SYMBaiEX
SYMBaiEX merged commit d4c18c0 into main Oct 26, 2026
7 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.

2 participants