Skip to content

[Feat] Consensus-restricted keywords.#2685

Merged
vicsn merged 11 commits intostagingfrom
feat/restrict-constructor
Apr 25, 2025
Merged

[Feat] Consensus-restricted keywords.#2685
vicsn merged 11 commits intostagingfrom
feat/restrict-constructor

Conversation

@d0cd
Copy link
Collaborator

@d0cd d0cd commented Apr 17, 2025

Motivation

As programming standards evolve on Aleo, it would be useful to be able to enforce syntactic restrictions at the consensus layer. This PR:

  • adds a mechanism for describing versioned syntax restrictions and enforces them in check_transactions.
  • adds a CI runner with the test feature enabled for the synthesizer crate.

Test Plan

A test was added verifying that syntax restrictions are not enforced before a given version height and that they are enforced after.

@d0cd
Copy link
Collaborator Author

d0cd commented Apr 17, 2025

@reviewers I'm not too happy with the name RESTRICTED_KEYWORDS, any ideas for a better one?

@d0cd d0cd requested review from raychu86 and vicsn April 17, 2025 00:18
@vicsn vicsn added the v3.7.0 label Apr 17, 2025
@acoglio
Copy link
Collaborator

acoglio commented Apr 17, 2025

RESERVED_KEYWORDS is perhaps more common?

Copy link
Collaborator

@vicsn vicsn left a comment

Choose a reason for hiding this comment

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

LGTM! Left two style suggestions.

synthesizer CI is failing when run with the --test feature because test_migration_v3_maximum_validator_increase assumes max validator set increases at consensus v3, whereas with the test feature enabled the max validator set is static at 100.

In other words, our usage of the test feature is a bit of an undocumented mess.

As a "quickfix", one option would be to use the same sample_finalize_state approach as in test_migration_v3_maximum_validator_increase, and then we can also get rid of introducing yet another CI job.

@d0cd
Copy link
Collaborator Author

d0cd commented Apr 17, 2025

RESERVED_KEYWORDS is perhaps more common?

I had mapped the concept of "reserved" keywords to the KEYWORDS array.
This feels more like syntactic recommendations to me.
Like linting...?

@d0cd
Copy link
Collaborator Author

d0cd commented Apr 17, 2025

As a "quickfix", one option would be to use the same sample_finalize_state approach as in test_migration_v3_maximum_validator_increase, and then we can also get rid of introducing yet another CI job.

If it's not too costly, it might be nice to have a CI run with the test feature enabled.
I noticed a few other tests (1, 2, 3, 4, 5) that also use it.

@d0cd d0cd requested a review from raychu86 April 18, 2025 14:14
@vicsn vicsn requested a review from niklaslong April 23, 2025 10:09
Copy link
Collaborator

@niklaslong niklaslong left a comment

Choose a reason for hiding this comment

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

LGTM overall, pending the changes requested by other reviewers.

@vicsn vicsn merged commit f57cecb into staging Apr 25, 2025
2 checks passed
@vicsn vicsn deleted the feat/restrict-constructor branch April 25, 2025 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants