Skip to content

feat: Support airbender delegations in precompiles#209

Open
popzxc wants to merge 14 commits into
mainfrom
popzxc-airbender-precompiles
Open

feat: Support airbender delegations in precompiles#209
popzxc wants to merge 14 commits into
mainfrom
popzxc-airbender-precompiles

Conversation

@popzxc

@popzxc popzxc commented Feb 24, 2026

Copy link
Copy Markdown
Member

This PR adds support for airbender delegations in precompiles, which is a prerequisite for making EraVM provable in airbender.

It does it through airbender-crypto crate from airbender-platfrom which is adapted from zksync-os dev branch with one exception: keccak delegation was taken from the rr/ethrpoofs branch.

Dependency on popzxc/airbender-platform with main branch is temporary -- before we merge this PR, I expect that we'll move the repo to the matter-labs org, have a release, and will pin via tag.

@0xVolosnikov 0xVolosnikov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes look ok, but left some comments.

But an important note: I don't think that we actually run these tests in CI. And we do not run anything in RISC-V context at all.

Additionally, imo keccak256 testing should be enhanced. It is true for all precompiles in general, but this is out of scope for this PR.

Comment thread crates/zk_evm_abstractions/src/precompiles/ecadd.rs Outdated
Comment thread crates/zk_evm_abstractions/src/precompiles/ecadd.rs Outdated
Comment thread crates/zk_evm_abstractions/src/precompiles/keccak256.rs Outdated
Comment thread crates/zk_evm_abstractions/src/precompiles/keccak256.rs Outdated
Comment thread Cargo.toml Outdated
Comment thread crates/zk_evm_abstractions/src/utils/airbender_bn254.rs Outdated
Comment thread crates/zk_evm_abstractions/src/precompiles/ecpairing.rs Outdated
Comment thread crates/zk_evm_abstractions/src/precompiles/ecrecover.rs Outdated
@0xVolosnikov

0xVolosnikov commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Looks fine, I'll take one more deeper manual review later, after we finalize other parts. I'll leave some AI-generated comments below

@0xVolosnikov 0xVolosnikov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Deep review pass. Three coverage/observability concerns + two trait-shape suggestions. Left inline.

Comment thread crates/zk_evm_abstractions/src/precompiles/keccak256/tests/utils.rs
Comment thread crates/zk_evm_abstractions/src/precompiles/ecadd/tests/mod.rs
Comment thread crates/zk_evm_abstractions/src/precompiles/ecrecover/mod.rs
Comment thread crates/zk_evm_abstractions/src/precompiles/keccak256/mod.rs
Comment thread crates/zk_evm_abstractions/src/precompiles/keccak256/mod.rs
@popzxc

popzxc commented Apr 20, 2026

Copy link
Copy Markdown
Member Author

@0xVolosnikov -- my pushback is not because I disagree with the feedback; it's just I believe that LLM-induced "improvement loop" can never end in practice. LLMs are good at things that can be criticized, but LLMs are still bad at deciding whether the critique is justified given context. I have had several rounds of LLM-based self-review before publishing myself with different prompting techniques.

I would classify provided LLM comments as "technically correct, practically low-impact" (again, given the purpose and original scope of the PR), so to me it sounds like "we've reached the point where LLM suggestions are not that useful, and a human review will provide much more value"; otherwise we might never merge this PR simply because the codebase is not perfect in the first place.

@0xVolosnikov

Copy link
Copy Markdown
Contributor

@popzxc sure, agree with you, feel free to ignore if you don't see any value.

@popzxc popzxc marked this pull request as ready for review April 23, 2026 12:28
@ninolipartiia

Copy link
Copy Markdown

@popzxc, flagging a potential issue for your review, please tell me if you think it's off base.

The following 5 Wycheproof vectors (tcIds 169, 205, 208, 222, 223) produce divergent verdicts between the two secp256r1_verify backends:

  • Legacy (RustCrypto p256) → Ok(false) (signature invalid, verify ran to completion)
  • Delegated (airbender-crypto) → Err(()) (structural reject)

At the precompile output these map to different memory writes ([1, 0] vs [0, 0])

/// All tracked backend divergences. Each is a concrete
/// `(digest, r, s, x, y)` on which `LegacySecp256r1Backend::verify` and
/// `DelegatedSecp256r1Backend::verify` produce different outcomes.
///
/// Current pattern for every entry: legacy returns `Ok(false)` (verification
/// ran to completion but failed), delegated returns `Err(())` (structural
/// reject). The precompile's memory-write layer maps these to different
/// output words (`[1, 0]` vs `[0, 0]`).
pub const KNOWN_DIVERGENCES: &[KnownDivergence] = &[
    KnownDivergence {
        tc_id: Some(169),
        label: "wycheproof-169: point at infinity during verify",
        digest_hex: "bb5a52f42f9c9261ed4361f59422a1e30036e7c32b270c8807a419feca605023",
        r_hex: "7fffffff800000007fffffffffffffffde737d56d38bcf4279dce5617e3192a8",
        s_hex: "555555550000000055555555555555553ef7a8e48d07df81a693439654210c70",
        x_hex: "b533d4695dd5b8c5e07757e55e6e516f7e2c88fa0239e23f60e8ec07dd70f287",
        y_hex: "1b134ee58cc583278456863f33c3a85d881f7d4a39850143e29d4eaf009afe47",
        legacy_outcome: Ok(false),
        delegated_outcome: Err(()),
    },
    KnownDivergence {
        tc_id: Some(205),
        label: "wycheproof-205: duplication bug",
        digest_hex: "bb5a52f42f9c9261ed4361f59422a1e30036e7c32b270c8807a419feca605023",
        r_hex: "6f2347cab7dd76858fe0555ac3bc99048c4aacafdfb6bcbe05ea6c42c4934569",
        s_hex: "bb726660235793aa9957a61e76e00c2c435109cf9a15dd624d53f4301047856b",
        x_hex: "5b812fd521aafa69835a849cce6fbdeb6983b442d2444fe70e134c027fc46963",
        y_hex: "7c75bf0c5c9f6d17ffb16d2726bf30a9c7aaf31a8d317472b1ea145ab66db616",
        legacy_outcome: Ok(false),
        delegated_outcome: Err(()),
    },
    KnownDivergence {
        tc_id: Some(208),
        label: "wycheproof-208: comparison with point at infinity",
        digest_hex: "bb5a52f42f9c9261ed4361f59422a1e30036e7c32b270c8807a419feca605023",
        r_hex: "555555550000000055555555555555553ef7a8e48d07df81a693439654210c70",
        s_hex: "3333333300000000333333333333333325c7cbbc549e52e763f1f55a327a3aa9",
        x_hex: "dd86d3b5f4a13e8511083b78002081c53ff467f11ebd98a51a633db76665d250",
        y_hex: "45d5c8200c89f2fa10d849349226d21d8dfaed6ff8d5cb3e1b7e17474ebc18f7",
        legacy_outcome: Ok(false),
        delegated_outcome: Err(()),
    },
    KnownDivergence {
        tc_id: Some(222),
        label: "wycheproof-222: public key shares x-coord with generator (low y)",
        digest_hex: "bb5a52f42f9c9261ed4361f59422a1e30036e7c32b270c8807a419feca605023",
        r_hex: "44a5ad0ad0636d9f12bc9e0a6bdd5e1cbcb012ea7bf091fcec15b0c43202d52e",
        s_hex: "249249246db6db6ddb6db6db6db6db6dad4591868595a8ee6bf5f864ff7be0c2",
        x_hex: "6b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c296",
        y_hex: "4fe342e2fe1a7f9b8ee7eb4a7c0f9e162bce33576b315ececbb6406837bf51f5",
        legacy_outcome: Ok(false),
        delegated_outcome: Err(()),
    },
    KnownDivergence {
        tc_id: Some(223),
        label: "wycheproof-223: public key shares x-coord with generator (high y)",
        digest_hex: "bb5a52f42f9c9261ed4361f59422a1e30036e7c32b270c8807a419feca605023",
        r_hex: "bb5a52f42f9c9261ed4361f59422a1e30036e7c32b270c8807a419feca605023",
        s_hex: "249249246db6db6ddb6db6db6db6db6dad4591868595a8ee6bf5f864ff7be0c2",
        x_hex: "6b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c296",
        y_hex: "b01cbd1c01e58065711814b583f061e9d431cca994cea1313449bf97c840ae0a",
        legacy_outcome: Ok(false),
        delegated_outcome: Err(()),
    },
];

@popzxc

popzxc commented Apr 24, 2026

Copy link
Copy Markdown
Member Author

@ninolipartiia thank you! Pushed a fix and added provided tests in f7048c2.
Checked if other precompiles are prone to similar errors -- they are not.

@ninolipartiia

Copy link
Copy Markdown

@popzxc, thanks, the fix looks good to me. Considering the tests from the wycheproof were added, shouldn't it be reflected in the license?

I'd suggest adding more tests from here for secp256r1_verify, and tests from here for ecrecover. Though it's optional, I already checked that they pass.

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.

3 participants