chore: remove library check for MAX_INPUTS_PER_NOTE
#2231
Conversation
mmagician
added
the
no changelog
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you. I left a couple of comments inline.
| # check that number of inputs is less than or equal to MAX_INPUTS_PER_NOTE | ||
| dup.1 push.MAX_INPUTS_PER_NOTE u32assert2.err=ERR_PROLOGUE_NOTE_INPUTS_LEN_EXCEEDED_LIMIT | ||
| u32lte assert.err=ERR_PROLOGUE_NOTE_INPUTS_LEN_EXCEEDED_LIMIT |
There was a problem hiding this comment.
I think we should still keep this check - the use case for this is different from what we do in the prologue. Specifically, this procedure is used to build note inputs for output notes. So, we still want to make sure here that the user doesn't compute invalid note input commitment.
| # Re-export the max inputs per note constant. | ||
| pub use ::miden::protocol::util::note::MAX_INPUTS_PER_NOTE |
There was a problem hiding this comment.
Let's move this to the top of the file - maybe after the "errors" section (create a new "constants" section).
Also, not related to this PR, but we should update the docs. Specifically, here we no longer have get_max_inputs_per_note procedure. Instead, we are exporting this constant.
|
@copilot address the comments in the PR review |
|
@mmagician I've opened a new pull request, #2242, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
superseded by #2232 |
This check is already done as part of the prologue logic:
miden-base/crates/miden-protocol/asm/kernels/transaction/lib/prologue.masm
Line 664 in e062f75
So the extra check at the library level is redundant.
Related to #2221 (comment)