-
Notifications
You must be signed in to change notification settings - Fork 21
Debug BLS
#773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Debug BLS
#773
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
(defconstraint pair-of-inputs-constancy () | ||
(if-not-zero ACC_INPUTS | ||
(if (will-remain-constant! ACC_INPUTS) | ||
(if-zero (- (next ACC_INPUTS) ACC_INPUTS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this rejected by go-corset ? The original version looked ok to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, they are probably equivalent, but I believe this way is more clear
(vanishes! (shift [OUTGOING_DATA 1] 7)) | ||
(eq! (shift [OUTGOING_DATA 2] 7) (prc---callee-gas)) | ||
(vanishes! (shift [OUTGOING_DATA 3] 7)) | ||
(eq! (shift [OUTGOING_DATA 4] 7) (prc-g1msm-prc-g2msm---precompile-cost))))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we replace all the (shift [OUTGOING_DATA X] Y)
with meaningful shorthand names ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but I would prefer in a separated PR.
(eq! (shift OUTGOING_INST 6) EVM_INST_DIV) | ||
(vanishes! (shift [OUTGOING_DATA 1] 6)) | ||
(eq! (shift [OUTGOING_DATA 2] 6) (prc---callee-gas)) | ||
(eq! (* (shift [OUTGOING_DATA 2] 6) (msm-pair-size)) (prc-g1msm-prc-g2msm---msm-cost-numerator_msm-pair-size)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: EVM Division Precompile Constraint Error
The compute-precompile-cost-integer-division
constraint misconfigures the EVM_INST_DIV
operation. OUTGOING_DATA 2
is incorrectly constrained as a value related to the quotient, but it should be the dividend (numerator). The actual division result (the precompile cost) should come from OUTGOING_RES_LO
.
(vanishes! (shift [OUTGOING_DATA 3] 6)) | ||
(eq! (* (shift [OUTGOING_DATA 4] 6) (msm-pair-size) PRC_BLS_MULTIPLICATION_MULTIPLIER) | ||
(prc-g1msm-prc-g2msm---precompile-cost_msm-pair-size_PRC_BLS_MULTIPLICATION_MULTIPLIER))))) | ||
(eq! (shift [OUTGOING_DATA 4] 6) PRC_BLS_MULTIPLICATION_MULTIPLIER)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Cost Calculation Error in Precompile Constraint
The prc-g1msm-prc-g2msm---compute-precompile-cost-integer-division
constraint performs a division but doesn't store the result into (shift OUTGOING_RES_LO 6)
. As a result, prc-g1msm-prc-g2msm---precompile-cost
will not reflect the computed value, leading to an incorrect precompile cost.
Note
Refactors BLS MSM precompile pricing/gas checks, raises MSM CT_MAX to 7, widens
OUTGOING_INST
/PRC_NAME
to i16, marks OOB add/mod lookups as unchecked, and adjusts constancy condition logic.oob/*/precompiles/common/bls/bls_msm.lisp
):reference-table-discount
; computesdiscount
conditionally onnum-inputs <= 128
.EVM_INST_DIV
,MOD_FLAG
=1,WCP_FLAG
=0); derivesprecompile-cost
at shift 6 and movesinsufficient-gas
to shift 7.precompile-cost
in a separate LT step; simplifiesreturn-gas
tocallee-gas - precompile-cost
(removes pair-size/multiplier scaling).OUTGOING_INST
to:i16
inoob/*/columns.lisp
.PRC_NAME
to:i16
inreftables/prague/bls_reftable.lisp
.CT_MAX_BLS_G1_MSM
/CT_MAX_BLS_G2_MSM
from 6 to 7 inoob/*/constants.lisp
.oob-into-add
andoob-into-mod
clookups as:unchecked
inoob/*/lookups/
.blsdata/*/generalities/constancy_conditions.lisp
):will-remain-constant! ACC_INPUTS
check withif-zero (- (next ACC_INPUTS) ACC_INPUTS)
before enforcingNONTRIVIAL_POP_BIT
constancy.Written by Cursor Bugbot for commit a9247c7. This will update automatically on new commits. Configure here.