Skip to content
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

Handle sums-of-products correctly in daml ledger export #15152

Merged
merged 6 commits into from
Oct 4, 2022
Merged

Conversation

akrmn
Copy link
Contributor

@akrmn akrmn commented Oct 3, 2022

This PR changes the encoding logic for values such that Variants with fields are encoded as valid Daml.

This works by adding a special case for the encoding of a Variant v whose value is a Record r, where r and v's package id and module name match and where r's entity name is equal to v's plus . and a suffix.

Closes #14723


Spun off as #15153:

Note that there's still a problematic case:

Variant(variantId = V, constructor = C, value = Unit)

This could represent a value of variant type V with constructor C, where C takes no arguments, e.g.

data V = A Int | B Text | C

var : V
var = C

or a value of variant type V a with constructor C a, where a ~ (), e.g.

data V a = A Int | B Text | C a

var : V ()
var = C ()

There is no way to tell these apart without the type information at hand. We could make the value field of Variant optional, leaving value unset for the former case and set as Unit for the latter, but it's currently commented as Required in https://github.com/digital-asset/daml/blob/main/ledger-api/grpc-definitions/com/daml/ledger/api/v1/value.proto#L146-L148

@akrmn akrmn requested review from a team October 3, 2022 14:24
@akrmn akrmn force-pushed the fix-14723 branch 4 times, most recently from be9653e to 39c3202 Compare October 3, 2022 15:10
Copy link
Collaborator

@remyhaemmerle-da remyhaemmerle-da left a comment

Choose a reason for hiding this comment

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

Nice. Thanks a lot.

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.

Handle sums of products correctly in daml ledger export
2 participants