Vendor bn128 silkpre precompiles

[ryankeleti]

Vendored code changed from silkpre to monad prefixes. We no longer technically need the silkpre git submodule, but it provides libsecp256k1 and libff that the vendored precompiles rely on. Once it is suitable to make libsecp256k1 a system dependency, we can also add libff as a git submodule and remove silkpre.

[Copilot]

Pull request overview

This PR vendors the alt_bn128 (EIP-196/EIP-197) precompile implementation from Silkpre into third_party/silkpre_vendor, switches the Ethereum precompile dispatcher over to the new monad_* entrypoints, and updates build/NOTICE metadata accordingly.

Changes:

• Add vendored bn128 precompile implementation (bn_add, bn_mul, snarkv) with monad-prefixed APIs and caller-provided output buffers.
• Update precompiles_impl.hpp to call the new bn128 functions instead of the previous silkpre/precompile.h wrappers.
• Wire bn128 into monad_crypto’s build and update third-party NOTICE text.

Verdict: NEEDS CHANGES

🤖 Generated with Claude Code

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
third_party/silkpre_vendor/src/category/crypto/silkpre_vendor/bn128.hpp	Adds monad-prefixed bn128 precompile API declarations.
third_party/silkpre_vendor/src/category/crypto/silkpre_vendor/bn128.cpp	Adds vendored libff-based bn128 implementation for add/mul/pairing.
third_party/silkpre_vendor/NOTICE	Documents bn128 derivation/source for licensing notice completeness.
category/execution/ethereum/precompiles_impl.hpp	Switches bn128 precompile implementations to the new vendored functions.
category/crypto/CMakeLists.txt	Builds/links the new bn128 sources into monad_crypto and adds libff linkage.

Suppressed comments (1)

third_party/silkpre_vendor/src/category/crypto/silkpre_vendor/bn128.cpp:186

• Same issue as monad_bn_add: std::basic_string<uint8_t> input(ptr, len); requires ptr to be non-null, so empty-input calls (len==0) are UB if the caller passes a null pointer. Construct/copy the buffer in a way that is safe when len == 0.

bool monad_bn_mul(uint8_t out[64], const uint8_t *ptr, size_t len) {
    std::basic_string<uint8_t> input(ptr, len);
    right_pad(input, 96);

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Reviewed the vendored bn128 code against upstream silkpre @ bbc3498e (lib/silkpre/precompile.cpp): the alt_bn128 logic — to_scalar, field-element range checks, G1/G2 decoding (including the G2 subgroup check), the zero-point special cases, LE→BE encoding, and the snarkv accumulator loop with the len % 192 guard — is a faithful copy with only the stated modifications (monad prefixes, caller-provided output buffer). The rewiring in precompiles_impl.hpp is correct: output sizes match what silkpre returned on success (64/64/32), failure paths leave the buffer untouched and from_impl_result frees it, and the malloc/memcpy/free round-trip per call is gone. Input clamping/right-padding semantics at the *_execute call sites are unchanged. The vendored header's #ifndef guard and formatting match the existing convention of the other silkpre_vendor files, NOTICE is updated, and check-trait-instantiations / clang-format / license checks are green.

One non-blocking finding posted inline: the silkpre library target link and its include directories in category/execution/CMakeLists.txt are now dead and can be dropped (the submodule itself is still needed for secp256k1/ff, as the description notes).

Verdict: CORRECT

🤖 Generated with Claude Code