-
Notifications
You must be signed in to change notification settings - Fork 59
Description
[ This issue was auto-migrated from DA's internal repo (DACH-NY/canton-network-node#18541). Original author: @moritzkiefer-da ]
The switch to requiring CIPs for every Daml change has made it much harder to develop Daml changes. We cannot merge Daml changes freely to main as this pins down the changes in a specific order and makes it hard to undo them if we need to.
Currently, the way we handle that is one of either:
- Merge to main and hope it gets accepted anyway.
- Keep it in a feature branch until the CIP is accepted.
1 is obviously risky and could backfire. 2 adds a lot of overhead on our side to manage potentially very long-running feature branches. We've experienced that first-hand with the governance vote changes and I really don't want to have to repeat that for everything that needs Daml changes.
What we'd really like is some sort of feature flag mode like we can easily have in our backend code that allows us to have the code in main, test it at least in integration tests and short lived clusters while ensuring that it will never get uploaded to dev/test/mainnet.
The best option I see is that we use some sort of Daml snapshot version. Concretely:
- Define a snapshot as a Daml package with a version "x.y.z.n" where I'd start
nat 100 just to make it very explicit that it is separate. Needs double checking that those are valid versions but I'm fairly sure they are (semverx.y.z-snapshot…is not). - Add a feature flag to the backends
supportDamlSnapshotVersionsthat is false by default. If it is false, snapshots will never be uploaded. If it is true, snapshots are considered like any other version and can be voted on and the latest snapshot is used as the default version. - Run integration tests and all short lived clusters with the feature flag enabled. Run cilr with the flag disabled (could perhaps do some fine tuning and disable the feature flag in some more places).
- The daml code in main usually has a snapshot version.
- When a CIP is approved we move the Daml code from the snapshot into a new stable version.
Moving Daml code from snapshot into a stable version
There is an easy case: If everything in the snapshot should be included in the next stable version you just change the version numbers to stable and you're done.
What I'm unsure about is how we would best tackle the case where we want to include part of the latest snapshot in a stable version but not everything. The best I can come up with is this:
- Undo the changes you don't want included, change the version numbers to stable and check in the DARs. Make a commit for that. Note that this commit will not compile for the backends in general as they have been written against the snapshot Daml models.
- Reset Daml code back to the snapshot version (but bump the
zinx.y.z.nand resetnback to 100).
This needs to be two separate commits. A reviewer could run damlDarLockFileCheck on the first commit to check that the checked in DARs are really correct and with some work we could (and should) also make CI recognize these changes (possibly through a special commit message) and do that automagically.
This is slightly awkward but in the end I expect this to be an infrequent procedure so I don't think it's too bad. We need to optimize for it being safe but not so much for it being convenient.
The part that is imho very awkward is merging it to main. You can either:
- Squash merge. But then you lose the Daml code for the stable versions. That's a big nono imho.
- Don't squash merge. Then you get the two commits separately which is nice so you do have the Daml code and can
damlDarLockFileCheck. The awkward part is that you need to disable CI for everything but Daml checks since as mentioned above the backend cannot be expected to compile against this. It's not hard to do but i don't like it. Can't come up with anything better though. Note that until we actually move development to splice this doesn't work for external verification. But given that this is the plan I would just assume this happens and not try to work around the fact that we don't mirror individual commits.
- Add config flag to not upload snapshot packages (default: false)
- Switch Daml packages to snaphot versions
- Adjust release infra to allow mutations of snapshot packages
- Add test that snapshot versions are not uploaded by default
- Add test that enables snapshot uploads and tests that they are uploaded
- Update readme to document how snapshot versions are used
- Add support for partially "unsnapshotting" the Daml code
Scenarios
- Additive changes: Examples are the 5% development fund and minting delegation
- All existing tests work with and without the change
- newly added tests (that need to have some version annotation anyway) will only work with the new version
- slight side note: we don't know the final version at the point when we add the test, would be nice to be able to tag them with "@NeedsDevelopmentFundCoupon
instead of @SpliceAmulet_0_1_42so you only need to change one place once it moves from snapshot to non-snapshot
- slight side note: we don't know the final version at the point when we add the test, would be nice to be able to tag them with "@NeedsDevelopmentFundCoupon
- Breaking changes: Examples are the changes to 24h signing delays, those will break txlog tests until the backend is adjusted
- however once adjusted the backend should be compatible with old and new again
- Ideal scenario: All tests use the non-snapshot version by default and you can gradually migrate
- This can work iff
- We unvet all packages at the beginning of each test
- there are no other changes in the snapshot that have tests and are not yet compatible with the change
- e.g. the 5% development fund tests cannot also assume working scan txlogs
- that seems like an ok limitation to accept
- This can work iff
Partial Unsnapshotting
Let's say the snapshot currently contains both the 5% development fund changes and the minting delegation changes.
The backend code requires both to compile. We now want to move the 5% development fund changes to a stable version
Ideal commits:
- First commit changes the Daml code to only contain the 5% development fund changes and Daml snapshot versions to stable versions
- Slightly weird: The diff is then a removal of the minting delegation changes rather than seeing the actual changes that get stabilized. That seems problematic, the whole point of snapshot versions is that on merging to main the code must not be final. But now you don't really have a review step for the final version. Ofc you can produce a diff manually but that is clunky.
- CI on this commit must
- compile the backend against the snapshot Daml models (otherwise it won't compile).
- Second commit adds back the 5% changes, CI on that runs normally.
Issues
- Reviewing this is a giant mess and seems to introduce a much higher possibility for errors and the Daml code not being in the shape you want it to be even with CI enforcement that you can't mess up the checked in DARs.
Options
- Accept this, rely on enough potentially manual diffs (maybe with links inserted automatically in comments through CI) in to review the final version of Daml code that gets stabilized to ensure it really is what you want to ship).
- Ignore partial unsnapshotting and only handle the case where you have a single change in main that you want to stabilize as a unit. This is technically easy. But you still have two issues:
2.1: You are still missing an explicit final review step (through GH, ofc you can create a manual diff against the last stable version and see all changes at once). I guess the answer is you have to be very diligent with adding TODOs that you can track. But it feels like a more risky model than the one we have right now where we essentially assume each change to main is fully shippable and review it to be so.
2.2: I'm not sure we are in a situation sufficiently often where we only have a single Daml change in-flight for this to actually be valuable.