Skip to content

[Feature] Introduce constructor, Operand::Checksum and Operand::Edition#2653

Closed
d0cd wants to merge 31 commits intostagingfrom
feat/constructors-and-operands
Closed

[Feature] Introduce constructor, Operand::Checksum and Operand::Edition#2653
d0cd wants to merge 31 commits intostagingfrom
feat/constructors-and-operands

Conversation

@d0cd
Copy link
Collaborator

@d0cd d0cd commented Mar 18, 2025

[NO NEW COMMITS ARE ALLOWED AS OF 5/27/25]

DO NOT MERGE WITHOUT AUDIT

Motivation

This PR introduces:

  • constructors to programs. Constructors are blocks of code that are run on-chain when finalizing a deployment.
  • Operand::Checksum. An operand that returns a field element containing the program's checksum. The checksum is defined as the BHP1024 hash over the program's bits.
  • Operand::Edition. The edition for the program. This is currently enforced to be N::EDITION (0) for all programs.

Note: The keyword constructor is now restricted after ConsensusVersion::V6 by this PR. This is enforced at the consensus-level as opposed to the program-level to ensure backwards compatibility.

Test Plan

Integration test cases were added to verify:

  • that a program is not deployed if its constructor fails.
  • a constructor can set a value in a mapping and that the new operands are functional.

Regression tests were re-ran to verify that the changes to the execution cost calculations do not affect existing programs.

@d0cd d0cd mentioned this pull request Mar 18, 2025
24 tasks
@d0cd d0cd requested review from raychu86 and vicsn March 19, 2025 04:05
let constructor_types = stack.get_constructor_types()?.clone();

// Initialize the finalize registers.
let mut registers = FinalizeRegisters::new(state, transition_id, *program_id.name(), constructor_types, nonce);
Copy link
Member

Choose a reason for hiding this comment

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

Auditors: The parameters used to initialize the FinalizeRegisters here should be reviewed for correctness and soundness.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@d0cd make sure to go over all unresolved comments and copy them over in the new PR you're making.

Copy link
Collaborator Author

@d0cd d0cd May 6, 2025

Choose a reason for hiding this comment

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

Gonna keep this PR, but I think we're all resolved in this one no?

@acoglio
Copy link
Collaborator

acoglio commented Mar 21, 2025

The parsing looks good. These are matching changes to the ABNF grammar:

operand = ...
        / program-id ; if not followed by "/"
        / [ program-id "/" ] %s"checksum"
        / [ program-id "/" ] %s"edition"
        / ...

constructor = cws %s"constructor" ws ":" *command

program = ...
          1*( constructor / mapping / struct / record / closure / function )
          ...

According to some discussion in this PR, we expect a change of the new rule to

constructor = cws %s"constructor" ws ":" 1*command

i.e. at least one command in a constructor.

Copy link
Collaborator

@acoglio acoglio left a comment

Choose a reason for hiding this comment

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

I just reviewed the parsing code.

Copy link
Collaborator

@ljedrz ljedrz left a comment

Choose a reason for hiding this comment

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

Reviewed the engineering aspects, left a suggestion.

@d0cd
Copy link
Collaborator Author

d0cd commented Mar 27, 2025

Can you make an issue drafting documentation for the constructor and new opcodes, ignoring program upgradability for now: https://github.com/aleoNet/welcome/issues/new

@vicsn since we'd like to do updatability all in one go, does it make more sense to add docs at once?

Base automatically changed from feat/cycle-protections to staging April 8, 2025 12:04
@d0cd d0cd requested review from raychu86 and vicsn April 8, 2025 13:33
@howardwu
Copy link
Member

@vicsn @d0cd -- pushing back on removing the reservation of keyword constructor.

It is easy to remove the restriction in the future, however it is difficult to introduce later.

I would advocate for adding it even sooner to "bookmark" it in an earlier release to this feature if possible.

@d0cd
Copy link
Collaborator Author

d0cd commented Apr 15, 2025

@vicsn @d0cd -- pushing back on removing the reservation of keyword constructor.

It is easy to remove the restriction in the future, however it is difficult to introduce later.

I would advocate for adding it even sooner to "bookmark" it in an earlier release to this feature if possible.

If it is to be added, then +1 on adding it sooner.
Currently, there are no programs on testnet or mainnet that contain the keyword constructor (scanned as of 12PM PST 4/15/25)
The process of reserving this keyword will be challenging. Some options included a (nearly) synchronous upgrade of quorum, or versioning the reserved keywords before and after an upgrade height. The fundamental issue is that there is nothing preventing the use of the keyword ATM.

@vicsn
Copy link
Collaborator

vicsn commented Apr 16, 2025

@vicsn @d0cd -- pushing back on removing the reservation of keyword constructor.

The process of reserving this keyword will be challenging. Some options included a (nearly) synchronous upgrade of quorum, or versioning the reserved keywords before and after an upgrade height. The fundamental issue is that there is nothing preventing the use of the keyword ATM.

The benefits of rushing in a synchronous upgrade are limited, let's add it to Consensus V6 (see the PR description for release dates). @d0cd can you make:

  • a snarkVM PR introducing the constructor keyword
    - a snarkOS PR disallowing deployments containing that keyword (similar as proposed here). Note that even if a deployment with that keyword exists before V6, users can continue to create executions for it, so its not detrimental to UX.

@d0cd
Copy link
Collaborator Author

d0cd commented Apr 17, 2025

The benefits of rushing in a synchronous upgrade are limited, let's add it to Consensus V6 (see the PR description for release dates). @d0cd can you make:

* a snarkVM PR introducing the `constructor` keyword

* a snarkOS PR disallowing **deployments** containing that keyword (similar as [proposed here](https://github.com/ProvableHQ/snarkOS/pull/3587#issuecomment-2788586686)). Note that even if a deployment with that keyword exists before V6, users can continue to create executions for it, so its not detrimental to UX.

See this PR.

@d0cd
Copy link
Collaborator Author

d0cd commented Jul 21, 2025

Closed in favor of #2807

@d0cd d0cd closed this Jul 21, 2025
@vicsn vicsn deleted the feat/constructors-and-operands branch August 28, 2025 15:43
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.

6 participants