-
Notifications
You must be signed in to change notification settings - Fork 204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix local contract upgrade #20296
base: main-2.x
Are you sure you want to change the base?
Fix local contract upgrade #20296
Conversation
934174c
to
c95b04e
Compare
sdk/daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/SBuiltin.scala
Outdated
Show resolved
Hide resolved
signatories = recomputed.signatories, | ||
observers = recomputed.observers, | ||
keyOpt = recomputed.keyOpt.map(_.globalKeyWithMaintainers), | ||
msg = errors.mkString("['", "', '", "']"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we do not want to keep the result of the two evaluations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created an issue and added a TODO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Thanks
b7ce1f0
to
67f14f3
Compare
67f14f3
to
e8a87f2
Compare
Co-authored-by: Remy <[email protected]>
e8a87f2
to
7498a33
Compare
Fixes #20282.
We now always perform the upgrade validation check in the engine, so the
NeedUpgradeVerification
question is effecively unused. I'll remove it in a latter PR as it will require quite some cleanup.I've removed test cases that test impossible situations if we assume the upload and type checkers have run, and which now fail because we try to import values.
I added a bunch of daml-script tests as a sanity check, but I don't cover the entire space (disclosed contracts, fetch, fetch by interface, etc.). I plan to do that as part of the "one true test" so I don't want to replicate too much work here.