Skip to content

Conversation

@amkCha
Copy link
Contributor

@amkCha amkCha commented Oct 30, 2025

  • make a command to list the addresses of precompiles (which is now a union of ranges)
  • add a PRC scenario column for P256_VERIFY
  • update CALL/precompile ≡ 1 in table unexcept and unaborted call type
  • update precompile related shorthands (common except id, common, weighted sum) + table recap
  • add P256_VERIFY to precompile classification liste + table
  • add P256_VERIFY to nsr and flag sum II def + table
  • explicit P256_VERIFY cannot have empty call data
  • add P256_VERIFY success, FKTH, FKTR case

Note

Introduces P256_VERIFY precompile across HUB/OOB/MMU with full scenarios, shorthands, and tables; unifies precompile address range and updates BLS/PRECOMPILE docs and diagrams.

  • Precompiles (HUB/OOB/MMU):
    • Add P256_VERIFY support: new scenario \scenPVerify, success/Failure (FKTH/FKTR) flows, result size, exo-phase wiring, and data transfer logic.
    • Extend common precompile processing and shorthands to include P256_VERIFY; enforce non-empty call data like BLS/point-evaluation.
    • Update OOB common handling to recognize \oobInstPVerify and related predictions.
  • Addressing and constants:
    • Introduce \prcAddressRange and switch references to it (incl. CALL handling and Tx-skip rules).
    • Define/normalize precompile address constants via \numConst and add \prcAddressPVerifyValue.
  • Scenarios, shorthands, and tables:
    • Update weighted sums (\scenPrecompileWeightedSum) to use address constants; add P256_VERIFY to classification, NSR/flag-sum defs, and tables.
    • Expand precompile scenario columns and recap tables to include P256_VERIFY.
  • Docs/diagrams:
    • Replace/introduce post-Cancun setup diagrams for fixed/variable-size precompiles; integrate P256_VERIFY alongside BLS.
    • Remove obsolete BLS-only representations; minor clarifications and typos fixed (e.g., "known" typos, OOB list adds CALL→precompile).

Written by Cursor Bugbot for commit dbf81e4. This will update automatically on new commits. Configure here.

@amkCha amkCha force-pushed the 272-eip-7951-secp256r1-scenario-columns-in-hub branch from dc56afa to b447b84 Compare November 5, 2025 14:55
@amkCha amkCha force-pushed the 272-eip-7951-secp256r1-scenario-columns-in-hub branch from dfbbe35 to e18c08d Compare November 10, 2025 18:18
@amkCha amkCha marked this pull request as ready for review November 10, 2025 18:18
@amkCha amkCha force-pushed the 272-eip-7951-secp256r1-scenario-columns-in-hub branch 2 times, most recently from 6b03928 to 8826b8f Compare November 12, 2025 13:35
@amkCha amkCha force-pushed the 272-eip-7951-secp256r1-scenario-columns-in-hub branch 3 times, most recently from a7aba74 to 3bd2df6 Compare November 12, 2025 14:04
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: Empty Call Data Check Inconsistency

The sanity check enforcing \locOobResultEmptyCallData = 0 is conditioned on \scenPrecompileCommonBls _{i} = 1, but \scenPVerify is not part of \scenPrecompileCommonBls. Since P256_VERIFY actively disallows empty call data (per ecadd_ecmul_ecpairing_bls_pverify/empty.tex), this constraint should also apply to P256_VERIFY but currently won't be enforced. The sanity check needs to include P256_VERIFY explicitly or be restructured to cover all precompiles that disallow empty call data.

hub/instruction_handling/call/precompiles/common/generalities.tex#L139-L145

anchorRow = i ,
relOffset = \prcCommonMiscRowOffset ,
sourceId = \cn_{i} ,
targetId = 1 + \hubStamp_{i} ,
sourceOffsetLo = \locPrcCdo ,
size = \locPrcCds ,
referenceOffset = 0 ,

Fix in Cursor Fix in Web


@amkCha
Copy link
Contributor Author

amkCha commented Nov 17, 2025

I may have missed it but there is a binarity constraints that should be added to \scenPVerify. Overall a lot of good stuff, a few comments are in place.

Okay, after checking, I think it's included here
image

Copy link
Collaborator

@OlivierBBB OlivierBBB left a comment

Choose a reason for hiding this comment

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

Only one concern about a macro containing numerals in its name. LGTM otherwise.

\def\columnPrcBlsPairingCheck {\verticalColumnName{\scenBlsPairingCheck }}
\def\columnPrcBlsMapFpToGOne {\verticalColumnName{\scenBlsMapFpToGOne }}
\def\columnPrcBlsMapFpTwoToGTwo {\verticalColumnName{\scenBlsMapFpTwoToGTwo }}
\def\columnPrcP256Verify {\verticalColumnName{\scenPVerify }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

It this accepted by Latex ? I believe you can't have numerals in latex macros.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed that it's underlining it in my IDE, but it's still showing nicely
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change it to be consistent with other naming

Copy link
Collaborator

Choose a reason for hiding this comment

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

Apparently it is. I had no idea ... how many macros I created would be improved by using numerals 😱

+ & \scenEcmul _{i} \\
+ & \scenEcpairing _{i} \\
+ & \scenPrecompileCommonBls _{i} \\
+ & \scenPVerify _{i} \\
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is ok. We may (not sure at this point) change this back as what we discovered this afternoon is that P256_VERIFY can only fail for OOG reasons, which are FKTH.

only (\emph{c}) requires touching \textsc{ram} in order to detect;
it is detected by \scenPrcFailureKnownToRam{};
if none of these conditions are triggered the output is a 32 byte integer slice $\textbf{o} \in \mathbb{B}_{32}$;
\end{description}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This we will have to change in light of our new understanding of P256_VERIFY

@OlivierBBB OlivierBBB self-requested a review November 19, 2025 20:02
@amkCha amkCha dismissed lorenzogentile404’s stale review November 19, 2025 20:08

All requested changed are done :)

@amkCha amkCha merged commit 09fb30e into main Nov 19, 2025
7 checks passed
@amkCha amkCha deleted the 272-eip-7951-secp256r1-scenario-columns-in-hub branch November 19, 2025 20:34
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.

4 participants