Skip to content

Support Ledger-friendly signatures in pallet-limit-orders#2966

Closed
UnArbosSix wants to merge 5 commits into
mainfrom
feat/limit-orders-ledger-signatures
Closed

Support Ledger-friendly signatures in pallet-limit-orders#2966
UnArbosSix wants to merge 5 commits into
mainfrom
feat/limit-orders-ledger-signatures

Conversation

@UnArbosSix

@UnArbosSix UnArbosSix commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Motivation

Limit orders currently accept only sr25519 signatures over the SCALE-encoded VersionedOrder. This change aims to broaden signature compatibility by accepting ed25519 signatures and signatures over a <Bytes>-wrapped Blake2-256 order hash.

Changes

  • Allow sr25519 and ed25519 MultiSignature variants during order validation while continuing to reject ECDSA.
  • Verify either the raw SCALE-encoded order or the <Bytes>-wrapped order hash.
  • Update the benchmark signing fixture to use the wrapped-hash form.
  • Add unit coverage for wrapped ed25519 signatures and ECDSA rejection.
  • Update the pallet documentation and runtime rejection test.

Files of interest are pallets/limit-orders/src/lib.rs, pallets/limit-orders/src/tests/auxiliary.rs, and pallets/limit-orders/src/benchmarking.rs.

Behavioral impact

Existing sr25519 orders remain valid. The accepted signature surface expands to ed25519 and wrapped-hash signatures; ECDSA remains unsupported. The order hash remains bound to the complete SCALE-encoded order.

Testing

The PR adds pallet-level coverage for wrapped ed25519 acceptance and ECDSA rejection and updates the benchmark fixture. End-to-end coverage using the supported Ledger client is still required.

@vercel

vercel Bot commented Jul 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
subtensor Ready Ready Preview, Comment Jul 22, 2026 11:08pm

Request Review

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: SAFE

VERY HIGH scrutiny: 20-day-old account with no public repositories; mitigated by repository write access, matching author/committer identity, and no listed Gittensor association. Branch targets main.

Static analysis found no signature bypass or malicious behavior. Both accepted payload forms remain bound to the complete SCALE-encoded order, the wrapped hash is derived internally from that order, ECDSA remains explicitly rejected, and the runtime specification version is bumped to 438.

Findings

No findings.

Conclusion

No security vulnerabilities were found in the current diff.


🔍 AI Review — Auditor (domain review)

VERDICT: 👎

UNKNOWN Gittensor association; repository write access, but the public account is under 30 days old with limited history.

The spec version is correctly bumped to 438, although the PR body still says 437. The overlapping PRs only share common files and are not competing implementations.

The new TypeScript test exercises an ed25519 software key, not the Ledger backend, so it does not resolve the producer-side incompatibility. No runtime execution was needed to confirm this interface mismatch.

Findings

Sev File Finding
HIGH pallets/limit-orders/src/lib.rs:623 Ledger still cannot produce this signature inline

Prior-comment reconciliation

  • 6d56b334: not addressed — The new end-to-end test signs with a generated software ed25519 key; no compatible LedgerSigner producer path was added.

Conclusion

The verifier supports the proposed signature format, but the repository’s Ledger signing path still cannot produce it. The PR therefore does not yet deliver its core Ledger-compatibility goal.


📜 Previous run (superseded)
Sev File Finding Status
HIGH pallets/limit-orders/src/lib.rs:623 Ledger cannot produce this raw-message signature ➡️ Carried forward to current findings
The new end-to-end test signs with a generated software ed25519 key; no compatible LedgerSigner producer path was added.

@github-actions github-actions Bot 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.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/limit-orders/src/lib.rs
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@github-actions github-actions Bot 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.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/limit-orders/src/lib.rs
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@github-actions github-actions Bot 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.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/limit-orders/src/lib.rs
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@github-actions github-actions Bot 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.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/limit-orders/src/lib.rs
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@UnArbosSix UnArbosSix mentioned this pull request Jul 23, 2026
@UnArbosSix UnArbosSix closed this Jul 23, 2026
@UnArbosSix
UnArbosSix deleted the feat/limit-orders-ledger-signatures branch July 23, 2026 21:43
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.

1 participant