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

Enhance Navigator's lf-value-json encoding to fully support DAML LF primitives #2361

Closed
leo-da opened this issue Jul 31, 2019 · 3 comments · Fixed by #2463
Closed

Enhance Navigator's lf-value-json encoding to fully support DAML LF primitives #2361

leo-da opened this issue Jul 31, 2019 · 3 comments · Fixed by #2463
Assignees
Labels
component/ledger Sandbox and Ledger API

Comments

@leo-da
Copy link
Contributor

leo-da commented Jul 31, 2019

Large Decimal values is one of the cases.
See #2519

@leo-da leo-da added the component/ledger Sandbox and Ledger API label Jul 31, 2019
@leo-da leo-da added this to the HTTP JSON API First Version milestone Jul 31, 2019
@leo-da leo-da changed the title Enhance Navigator's lf-value-json encoding to fully support DAML primitives Enhance Navigator's lf-value-json encoding to fully support DAML LF primitives Jul 31, 2019
@digital-asset digital-asset deleted a comment from leo-da Aug 8, 2019
@S11001001 S11001001 self-assigned this Aug 8, 2019
@S11001001 S11001001 added the wip-issue This issue is being worked on. Use draft PRs for work in progress PRs. label Aug 8, 2019
@S11001001
Copy link
Contributor

banker's rounding

In most contexts, we expect LF values to have been pre-rounded, and don't do rounding again in order to avoid the risk of discarding data. That includes the Time.Timestamp LF Scala type underlying the LF Value ADT.

For the JSON format, we state:

Numbers must be within the bounds of Decimal, [–(10³⁸–1)÷10¹⁰, (10³⁸–1)÷10¹⁰]. Numbers outside those bounds will be rejected. Numbers inside the bounds will always be accepted, using banker's rounding to fit them within the precision supported by Decimal.

@S11001001
Copy link
Contributor

non-Z timestamps

Hypothetically, we could read timezones in the timestamp format. Instead, we only permit Z, and moreover require that Z is specified.

The UTC timezone designator must be included. The rationale behind the inclusion of the timezone designator is minimizing the risk that users pass in local times.

@S11001001
Copy link
Contributor

nested Optional

The Optional rules provide a consistent core rule for nested optionals:

  1. the innermost Optional (i.e. any optional without an optional directly therein) doesn't have a marker. null or absence (in the case of a surrounding [] for containing optional) means None.
  2. [] is a valid Some None.

This is the ruleset for the Optional section, but some other encodings assume something incompatible. For example, [42] is never a valid Optional Int under this scheme, nor is [] a valid None.

@S11001001 S11001001 removed the wip-issue This issue is being worked on. Use draft PRs for work in progress PRs. label Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/ledger Sandbox and Ledger API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants