feat(near): add signature verification tests + MetaMask provenance pin - #429
Open
shahan-khatchadourian-anchorage wants to merge 2 commits into
Open
Conversation
Copilot started reviewing on behalf of
shahan-khatchadourian-anchorage
July 30, 2026 22:59
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Adds missing unit-test coverage for NEAR Intents signature verification (verify.rs) to ensure both native ed25519 verification and ERC-191 (secp256k1) recovery-based verification behave correctly off-chain, including a provenance pin against a published MetaMask reference signature.
Changes:
- Added a comprehensive
#[cfg(test)]module forpresets/intents/verify.rs, covering valid/invalid ed25519 signatures, ERC-191 recovery behavior under tampering, and invalid recovery-id handling. - Introduced pinned fixture vectors for raw_ed25519 and deterministically generated ERC-191 signed payloads, including a self-consistency check against the generator.
- Added
k256as a dev-dependency (test-only) and updated the lockfile accordingly.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/chain_parsers/visualsign-near/src/presets/intents/verify.rs | Adds signature verification/recovery tests, deterministic ERC-191 vector generator, and MetaMask reference signature pin. |
| src/chain_parsers/visualsign-near/tests/fixtures/_vector_raw_ed25519.input | Adds a pinned raw_ed25519 signed test vector fixture. |
| src/chain_parsers/visualsign-near/tests/fixtures/_vector_erc191.input | Adds a pinned ERC-191 signed test vector fixture matching the in-test generator. |
| src/chain_parsers/visualsign-near/Cargo.toml | Adds k256 as a dev-dependency for deterministic secp256k1 test signing. |
| src/Cargo.lock | Records the added dev-dependency resolution (k256 and its transitive deps). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
shahan-khatchadourian-anchorage
force-pushed
the
shahankhatchadourian/near-a4-verify
branch
from
July 31, 2026 10:45
d7709ae to
4766586
Compare
Ports the dedicated signed-vector test coverage for signature verification: a raw_ed25519 test vector copied verbatim from an existing, published test suite for the intents protocol, a deterministically-generated ERC-191 vector (fixed test key, RFC6979 signing via k256), and tests for the tamper case, the invalid-recovery case, and the ERC-191 recover-not-reject semantics. New work beyond the port: erc191_generator_reproduces_metamask_reference_signature pins this crate's ERC-191 signing path (keccak256 personal_sign prehash + k256::ecdsa::SigningKey::sign_prehash_recoverable) against a private key, message, and signature published in the intents protocol's own erc191 crate test suite -- a signature a real MetaMask wallet actually produced. Reproducing those exact bytes gives the generated vector real-wallet-equivalent provenance without a wallet in the loop. k256 added as a dev-dependency only; bs58 (already a dependency) is now exercised by these tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A3's review-comment fix upstream changed verify_and_extract's second return value from Option<DefusePayload<..>> to Result<DefusePayload<..>, String> (so callers can surface why extraction failed instead of silently omitting the envelope/intents). This test predates that change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
shahan-khatchadourian-anchorage
force-pushed
the
shahankhatchadourian/near-a4-verify
branch
from
July 31, 2026 10:52
4766586 to
a337cd3
Compare
shahan-khatchadourian-anchorage
marked this pull request as ready for review
July 31, 2026 10:58
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fourth PR of the NEAR-consolidation stack (stacked on #428). Adds the dedicated test coverage for signature verification that the intents preset's
verify.rswas missing since A3 (which ported only the functions themselves, sincerender.rsneeds them to compile).Stack: A1 (#425) -> A2 (#426) -> A3 (#428) -> A4 (this) -> B (identity + wiring) -> C (docs site).
What this PR does
Adds
chain_parsers/visualsign-near/src/presets/intents/verify.rs's test module:valid_signature_verifies_natively/tampered_signature_is_invalid: a raw_ed25519 signed vector (tests/fixtures/_vector_raw_ed25519.input) copied verbatim from an existing, published test suite for the intents protocol, proving native verification runs correctly and fails closed (not panics) on tampering.tests/fixtures/_vector_erc191.input, fixed test key, RFC6979 signing viak256), a self-consistency test against the committed fixture, a valid-signature test, a tampered-payload test (ERC-191 "verification" is recovery, not pass/fail -- tampering recovers a different key, doesn't error), and an invalid-recovery-byte test proving the recovery-id guard actually prevents the nativeecrecoverpanic it exists for.New work: MetaMask provenance pin
erc191_generator_reproduces_metamask_reference_signaturetakes a private key, message ("Hello world!"), and expected 65-byte signature -- all copied from the intents protocol's ownerc191crate test suite, where they're documented as a signature a real MetaMask wallet produced -- and asserts this crate's own ERC-191 signing path (keccak256 personal_sign prehash +k256::ecdsa::SigningKey::sign_prehash_recoverable) reproduces those exact bytes.This gives the generated ERC-191 vector real-wallet-equivalent provenance without a wallet in the loop: since MetaMask/ethereumjs and this crate's
k256-based signer both use deterministic ECDSA (RFC6979), the same key + message must produce byte-identical output regardless of which implementation computes it. Passed on the first run.The private key is a long-published, throwaway test key from that external test suite, not a secret generated or exposed by this PR.
Verification
cargo test -p visualsign-near-- 52/52 pass.cargo clippy -p visualsign-near --all-targets -- -D warnings,cargo fmt --check-- clean.cargo check/clippy -p parser_app --no-default-features --features near-- clean (narrow-build variant).make -C src lint,make -C src test-- clean (default features; stock builds unaffected).ecrecoverpanic path it prevents.#[cfg(test)]-scoped withk256added only as a dev-dependency -- zero production attack surface -- and confirmed the private key is a pre-existing public test vector, not a leaked credential.