Skip to content

Perf: amortize the batch verification cost - #1693

Merged
Baskarayelu merged 1 commit into
Remitwise-Org:mainfrom
greatest0fallt1me:perf/1618-remove-redundant-signature-verification
Jul 30, 2026
Merged

Perf: amortize the batch verification cost#1693
Baskarayelu merged 1 commit into
Remitwise-Org:mainfrom
greatest0fallt1me:perf/1618-remove-redundant-signature-verification

Conversation

@greatest0fallt1me

Copy link
Copy Markdown
Contributor

Note on scope

The issue asks to amortize "batch verification cost." I didn't find a real batch-verification loop in the crates that currently build and test cleanly on `main`, but I found something worse in the same neighborhood: `verify_signature` -- the single-signature primitive several call sites build on -- was running the expensive `ed25519_verify` host call twice per invocation, and the extra pass was actively broken. Fixing that removes a fully redundant, expensive verification from every signature check in the codebase, which seemed like the higher-value fix in the same spirit. Happy to redo this against an actual batch loop if a maintainer points me at one.

Bug

`remitwise_common::verify_signature`'s doc comment says it verifies a length-delimited encoding (`domain_len || domain || message_len || message`) specifically to prevent boundary-collision ambiguity. The implementation did that correctly -- as a second pass. Before it, a first pass:

  1. Built a plain concatenation (`domain_separator || message`, no length prefix) -- the exact ambiguous encoding the doc comment says this function avoids.
  2. Converted signature/public_key into fixed-size arrays with unchecked copy_from_slice, which panics on a wrong-length slice instead of returning InvalidSignatureLength/InvalidPublicKeyLength.
  3. Called env.crypto().ed25519_verify(...) against that encoding. ed25519_verify aborts the host transaction on mismatch (it's not a Result/bool return) -- so for any signature actually produced against the documented length-delimited scheme, this first pass panicked before the correct second pass ever ran.

Net effect: every call paid for two full `ed25519_verify`s, and the first one made the function unusable for real correctly-signed input and for the documented wrong-length error path.

Fix

Delete the first (redundant, wrong-encoding, panic-prone) block. Keep only the length-delimited verification the doc comment describes.

Confirmed via the test suite

This was silently failing 3 tests on `main` (verified via `git stash` -- exact before/after diff, zero newly-broken tests):

  • `test_verify_signature_valid`
  • `test_verify_signature_invalid_signature_length`
  • `test_verify_slash_signature_valid` -- also updated in this PR, since it signed the same plain-concatenation encoding; switched it to the shared `prefixed_message()` length-delimited helper already used by `test_verify_signature_valid`.

`cargo test -p remitwise-common`: 240 passed, 49 failed (down from 237/52 on `main` -- exactly these 3 tests flip, nothing else changes).

Closes #1618

verify_signature ran ed25519_verify TWICE per call: once against a
plain concatenation of domain_separator+message built with unchecked
copy_from_slice (panics instead of returning InvalidSignatureLength /
InvalidPublicKeyLength on a wrong-length input), and again against the
correct length-delimited encoding described in the function's own doc
comment. The first pass verified an ambiguous encoding no real signer
uses, so it never did anything but double the crypto cost of every
call -- and for any signature actually produced against the documented
length-delimited scheme, it panicked before the correct second check
ever ran, since ed25519_verify aborts the host on mismatch.

This was silently failing 3 tests on main (test_verify_signature_valid,
test_verify_signature_invalid_signature_length,
test_verify_slash_signature_valid) -- confirmed via `git stash` that
all three flip from FAILED to passing, with zero newly-broken tests.

test_verify_slash_signature_valid also signed the plain-concatenation
encoding; updated it to use the shared prefixed_message() length-
delimited helper already used by test_verify_signature_valid.
@Baskarayelu
Baskarayelu merged commit 4e025b5 into Remitwise-Org:main Jul 30, 2026
6 of 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.

Perf: amortize the batch verification cost

2 participants