Enforce usage of latest program edition#29027
Conversation
JoshuaBatty
force-pushed
the
josh/program_edition
branch
from
5d5f864 to
13327fa
Compare
JoshuaBatty
force-pushed
the
josh/program_edition
branch
from
aff6f46 to
8d3c47f
Compare
JoshuaBatty
requested review from
d0cd,
mitchmindtree,
mohammadfawaz and
vicsn
vicsn
left a comment
vicsn
left a comment
There was a problem hiding this comment.
Thx! Minor fix required I think
leo/cli/commands/run.rs
Outdated
| // For local programs not yet deployed, default to edition 0 (first deployment). | ||
| // Network programs should already have their edition set by load_latest_programs_from_network. | ||
| // Note: The V8 constructor check doesn't apply here since `run` is local evaluation only. | ||
| let edition = edition.unwrap_or(0); |
There was a problem hiding this comment.
Nit. It could be nice to tell users when we're defaulting. Ditto for synthesize.
There was a problem hiding this comment.
0b0289e local programs now use LOCAL_PROGRAM_DEFAULT_EDITION (defaulting to 1) which keeps the logic clean and documented in one place.
There was a problem hiding this comment.
I made a print function here so we still inform the user. e1db033
so now you would see something like:
- credits.aleo (already included)
- some_network_dep.aleo (edition: 3)
- my_local_program.aleo (local)
vicsn
left a comment
vicsn
left a comment
There was a problem hiding this comment.
Thank you for making progress on this badly documented part of the code!
I find myself still being confused and lacking knowledge about how local v.s. network dependencies behave across commands (deploy, execute, repeated commands, etc.), so I'll leave further review to the others.
However, on that note I would really appreciate:
- either a design doc on how we can improve on that abstraction (CC my half-assed attempt #28750)
- or if you think it is optimal, an improvement to the documentation https://docs.leo-lang.org/guides/dependencies
There was a problem hiding this comment.
Thank you for your patience with this PR @JoshuaBatty!
The edition patch was a band-aid fix that I had made and it has come to bite us.
I thought about it for a while and I think it is actually okay to default the local program to edition 0.
I was concerned about this check in snarkVM, however, that's only run during transaction verification and uses the verifier's (the network) edition.
Since we don't verify the transaction when running Leo execute, we won't run into this error.
We should still warn users when we're defaulting so that they know.
@vicsn do you agree? Is there any other check on the VM side that I might be missing?
Otherwise this PR is really solid! It wrangles a lot of the complexity that was spread across a lot of the commands.
|
Thanks @d0cd! I ran an experiment to verify: Switched to edition 0 and ran the integration tests - snarkVM does reject edition 0 programs without constructors during local execution, not just transaction verification: Cannot execute some_sample_leo_program.aleo on edition 0 So the options are:
I'm inclined to stick with edition 1 since it's less friction for devs - but wanted to confirm that's the direction you'd prefer before we finalise. @vicsn Happy to look into documentation improvements once we've landed on a final approach here. |
|
|
Thanks for reviews everyone! @vicsn I've just opened a PR here to help clarify how local vs network dependencies work, caching behavior, and edition handling. Keen to see how we can improve this situation further with further CLI improvements and possibly introducing a .lock file in the future as per our discussion last night. |
Default local programs to edition 1 for run/execute/synthesize commands. Fetch actual editions from network for dependencies. Apply V8 constructor check only during deploy/upgrade. Closes #28983
JoshuaBatty
force-pushed
the
josh/program_edition
branch
from
7324c51 to
54212c0
Compare
3 participants
Summary
Closes #28983
This PR improves how Leo handles program editions, ensuring we use actual editions from the network rather than arbitrary defaults.
Changes
Edition Handling by Command
runexecutesynthesizedeployupgradeWhy edition 1 for local programs?
Local programs haven't been deployed yet, so they don't have a real on-chain edition. We default to edition 1 to avoid snarkVM's V8+ check that rejects edition 0 programs without constructors. This keeps local development frictionless.
V8 Constructor Check
The
check_edition_constructor_requirementsvalidation is only applied duringdeployandupgrade- the commands that actually interact with on-chain programs. If a network dependency is at edition 0 without a constructor, we error early with a clear message explaining the program needs to be upgraded on-chain.