(*BatchVerifier).Add arguments are the opposite of Verify

Verify(publicKey ed25519.PublicKey, message, sig []byte) bool
(v *BatchVerifier) Add(publicKey ed25519.PublicKey, sig, message []byte) bool

Note how Verify takes first the message and then the signature, while Add does the other way around.

This looks extremely error prone.

Indeed, BenchmarkVerifyBatch itself is using the function wrong, but still passes because it does not check the return value of Add. This also means that all benchmarks are testing a batch size of 0.

v.Add(pub, msg, ed25519.Sign(priv, msg))

As an API safety note, I think Verify should be documented to return false on a batch of size zero. Also, Add should return an error value to give a chance to errcheck-like linters to catch the fact that the error is not handled. Actually, it should not return anything and defer all results to Verify, so that if invalid inputs are inserted in the batch, Verify will fail. This is too big of a footgun, and if people want to optimize for malformed signatures they can just implement the length check themselves.

Since swapping the arguments is not going to break the API, I would recommend intentionally introducing a backwards-incompatible change, so that there aren't two compatible versions that work very differently. For example you could rename VerifyBatch to Verify, since BatchVerifier.VerifyBatch is redundant anyway.

P.S. I don't think Verify should purge the batch. There is no expensive resource to reuse, it's not documented, and it's a footgun: I would expect two sequential Verify operations to return the same value.

    // purge BatchVerifier for reuse
    v.entries = []ks{}

hdevalence / ed25519consensus

(*BatchVerifier).Add arguments are the opposite of Verify #9