stable encoding #21
Merged
stable encoding #21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rsoeldner
commented
Oct 19, 2023
| encodeDecimal d@(Decimal _ mantissa) | ||
| | isSafeInteger mantissa = J.build $ J.Aeson @Scientific $ fromRational $ toRational d | ||
| | otherwise = J.object [ "decimal" J..= T.pack (show d) ] | ||
| encodeUnit = J.object ["unit" J..= T.empty] -- TODO: Discuss? |
There was a problem hiding this comment.
@jmcardon How should we handle unit
There was a problem hiding this comment.
I think the empty text is fine 👍
rsoeldner
commented
Oct 19, 2023
| -- | Stable encoding of `CapabilityGuard FullyQualifiedName PactValue` | ||
| instance J.Encode (StableEncoding (CapabilityGuard FullyQualifiedName PactValue)) where | ||
| build (StableEncoding (CapabilityGuard name args)) = J.object | ||
| [ "cgPactId" J..= (error "a" :: T.Text) -- TODO: Check availability |
rsoeldner
commented
Oct 19, 2023
| instance J.Encode (StableEncoding ModRef) where | ||
| build (StableEncoding (ModRef mn imp _ref)) = J.object | ||
| [ "refSpec" J..= J.Array (StableEncoding <$> imp) | ||
| -- , "refInfo" J..= _modRefInfo o |
rsoeldner
commented
Oct 19, 2023
| PGuard g -> J.build (StableEncoding g) | ||
| PObject o -> J.build (StableEncoding o) | ||
| PModRef mr -> J.build (StableEncoding mr) | ||
| PCapToken _ct -> error "not implemented" |
imalsogreg
reviewed
Oct 19, 2023
|
|
||
| -- | | ||
| -- | ||
| -- Stable encoding which matches Pacts StableEncoding. |
There was a problem hiding this comment.
Some questions that could be addressed as documentation here in the haddock header:
- What is this stable encoding for?
- Where will the encoded results end up?
- How stable does the encoding need to be? vis. the following question
- What happens if we find a bug in the encoding and need to fix it?
- Does this encoding constitute a public API? Will users/clients consume/produce values in this encoding, or is it internal?
There was a problem hiding this comment.
In Robert's absence, I can answer some of these, and maybe @rsoeldner can include this in the haddocks in a separate PR.
- Stable encoding is for hashes and anything that produces an identifier with hashes, that is
principals,pactIds, etc. - The encoded results may be pactIds, or even semantic values produced by
hash - The encoding must match production. I've ported over some tests on
hash.replbut this is not enough, soon with the new module hashing, we will use repl tests as goldens to ensure some of our hashes are computably stable. - There should not be bugs in the encoding. It has to match prod 1-1 and we have to enforce this long term. There cannot be bugs in this code when we launch core to the world.
- This encoding may possibly be an API, but we're not even sure what core's API will be like yet. We cross that bridge when we get there, I think.
0xd34df00d
approved these changes
Oct 19, 2023
jmcardon
approved these changes
Oct 19, 2023
| encodeDecimal d@(Decimal _ mantissa) | ||
| | isSafeInteger mantissa = J.build $ J.Aeson @Scientific $ fromRational $ toRational d | ||
| | otherwise = J.object [ "decimal" J..= T.pack (show d) ] | ||
| encodeUnit = J.object ["unit" J..= T.empty] -- TODO: Discuss? |
There was a problem hiding this comment.
I think the empty text is fine 👍
|
|
||
| -- | | ||
| -- | ||
| -- Stable encoding which matches Pacts StableEncoding. |
There was a problem hiding this comment.
In Robert's absence, I can answer some of these, and maybe @rsoeldner can include this in the haddocks in a separate PR.
- Stable encoding is for hashes and anything that produces an identifier with hashes, that is
principals,pactIds, etc. - The encoded results may be pactIds, or even semantic values produced by
hash - The encoding must match production. I've ported over some tests on
hash.replbut this is not enough, soon with the new module hashing, we will use repl tests as goldens to ensure some of our hashes are computably stable. - There should not be bugs in the encoding. It has to match prod 1-1 and we have to enforce this long term. There cannot be bugs in this code when we launch core to the world.
- This encoding may possibly be an API, but we're not even sure what core's API will be like yet. We cross that bridge when we get there, I think.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.