Skip to content

Conversation

@Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Sep 22, 2025

Description

Proposal neo-project/proposals#205 . Adds a bn254 native contract implementation that mirrors the Solidity/EVM precompile interface.
The contract now routes elliptic curve add, scalar multiplication, and pairing through Nethermind’s
MCL bindings, handling Ethereum’s big-endian payload layout and zero-on-error semantics. Unit
coverage includes the canonical Ethereum GeneralStateTests pairing vectors to ensure cross-chain
compatibility. The native manifest snapshot was refreshed so genesis exposes the new entry points.

Fixes # (issue)

Type of change

  • Optimization (the change is only an optimization)
  • Style (the change is only a code style for better maintenance or standard purpose)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as
    expected)
  • This change requires a documentation update

How Has This Been Tested?

  • dotnet test tests/Neo.UnitTests/Neo.UnitTests.csproj --filter CryptoLib

Test Configuration:

  • OS: same as CI image
  • .NET SDK: 9.0.301
  • Tests reused Ethereum fixtures (GeneralStateTests/stZeroKnowledge/ecpairing_inputs.json, commit
    c67e485ff8b5be9abc8ad15345ec21aa22e290d9)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@Jim8y Jim8y force-pushed the feature/bn254-curve-support branch from c796a25 to 3188ee3 Compare September 22, 2025 03:59
@cschuchardt88
Copy link
Member

BouncyCastle can do the curve

public static ECDomainParameters GetBN254G1DomainParameters()
{
    // Define the prime modulus 'p' of the base field.
    // p = 21888242871839275222246405745257275088696311157297823662689037894645226208583
    BigInteger p = new BigInteger("21888242871839275222246405745257275088696311157297823662689037894645226208583");

    // Define the curve equation y^2 = x^3 + a*x + b.
    // For BN254, a = 0 and b = 3.
    BigInteger a = BigInteger.Zero;
    BigInteger b = new BigInteger("3");

    // Define the order of the G1 group 'n'.
    // n = 21888242871839275222246405745257275088548364400416034343698204186575808495617
    BigInteger n = new BigInteger("21888242871839275222246405745257275088548364400416034343698204186575808495617");
    // The cofactor 'h' is usually a small integer. For BN254, it is (p+1-t)/n.
    BigInteger h = BigInteger.One;

    // Construct the finite field F_p.
    ECCurve curve = new FpCurve(p, a, b, n, h);

    // Define the G1 generator point coordinates (x, y).
    // x = 1, y = 2.
    BigInteger gx = new BigInteger("1");
    BigInteger gy = new BigInteger("2");

    // Create the ECPoint for the G1 generator.
    ECPoint g = curve.CreatePoint(gx, gy);

    // Combine parameters into an ECDomainParameters object.
    return new ECDomainParameters(curve, g, n, h);
}

Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need some more time to verify the math.

{
partial class CryptoLib
{
[ContractMethod(Hardfork.HF_Gorgon, CpuFee = 1 << 19)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Faun? I thought we need to include them into the upcoming release.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe no hard fork is needed.

partial class CryptoLib
{
[ContractMethod(Hardfork.HF_Gorgon, CpuFee = 1 << 19)]
public static byte[] Bn254Add(byte[] input)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also need bn254Equal.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as bn254Serialize and bn254Deserialize.

return BN254.Add(input);
}

[ContractMethod(Hardfork.HF_Gorgon, CpuFee = 1 << 19)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[ContractMethod(Hardfork.HF_Gorgon, CpuFee = 1 << 19)]
[ContractMethod(Hardfork.HF_Gorgon, CpuFee = 1 << 21)]

Follow bls12381Mul, multiplication is more complicated.

ReadOnlySpan<byte> xBytes = encoded[..FieldElementLength];
fixed (byte* ptr = xBytes)
{
if (Mcl.mclBnFp_setBigEndianMod(ref point.x, (nint)ptr, (nuint)xBytes.Length) != 0)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call, in the case when the input has a non-canonical representation, takes the result modulo p (https://github.com/herumi/mcl/blob/master/include/mcl/bn.h#L310-L313), is this behavior correct and safe?

@Wi1l-B0t
Copy link
Contributor

No progress?

@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 23, 2025

I’ve reworked the BN254 native contract so it no
longer relies on Nethermind.MclBindings—the entire G1/G2 arithmetic and
Miller loop now live in BN254.Managed.* and are backed by BouncyCastle.
The wrapper in BN254.cs uses those managed routines, and Neo.csproj no
longer references the Nethermind package. I also updated the docs and tests to
reflect the EVM-style 32-byte success word, and the pairing vectors all pass
(dotnet test tests/Neo.UnitTests/Neo.UnitTests.csproj). Please take another
look.

@erikzhang
Copy link
Member

@neo-project/ngd-shanghai Need testing.

@shargon shargon requested a review from Copilot October 24, 2025 08:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds BN254 elliptic curve cryptography support to the Neo native CryptoLib contract, mirroring the Ethereum/EVM precompile interface. The implementation enables elliptic curve addition, scalar multiplication, and pairing operations through three new contract methods (bn254Add, bn254Mul, bn254Pairing) that handle Ethereum's big-endian payload layout and zero-on-error semantics. Unit tests include canonical Ethereum GeneralStateTests pairing vectors to ensure cross-chain compatibility.

Key Changes

  • Adds three BN254 native contract methods with Ethereum-compatible interfaces
  • Implements managed field arithmetic (Fp, Fp2, Fp6, Fp12) and curve operations for BN254
  • Includes comprehensive test coverage using Ethereum test vectors

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/Neo.UnitTests/SmartContract/Native/UT_NativeContract.cs Updates native contract manifest snapshot to include new bn254 methods
tests/Neo.UnitTests/SmartContract/Native/UT_CryptoLib.cs Adds comprehensive unit tests for BN254 operations using Ethereum test vectors
src/Neo/SmartContract/Native/CryptoLib.BN254.cs Implements the three public BN254 native contract methods with input validation
src/Neo/Cryptography/BN254.cs Core BN254 implementation handling Add, Mul, and Pairing operations with BouncyCastle integration
src/Neo/Cryptography/BN254.Managed.Fields.cs Implements field arithmetic for Fp and Fp2
src/Neo/Cryptography/BN254.Managed.Extensions.cs Implements extension field arithmetic for Fp6 and Fp12
src/Neo/Cryptography/BN254.Managed.Curves.cs Implements curve point operations and Miller loop for pairing
docs/native-contracts-api.md Documents the three new BN254 contract methods

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +43 to +44
if (input.Length % BN254.PairInputLength != 0)
throw new ArgumentException("Invalid BN254 pairing input length", nameof(input));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems already checked in Pairing method

Comment on lines +23 to +24
if (input.Length != BN254.G1EncodedLength * 2)
throw new ArgumentException("Invalid BN254 add input length", nameof(input));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this validation to Add method?

"2c145edbe7fd8aee9f3a80b03b0b1c923685d2ea1bdec763c13b4711cd2b8126",
"05b54f5e64eea80180f3c0b75a181e84d33365f7be94ec72848a1f55921ea762");

internal static bool IsAllZero(ReadOnlySpan<byte> data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ByteExtensions.NotZero?

@neo-project neo-project deleted a comment from Copilot AI Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants