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

Add missing fee and hash functions #28

Open
wants to merge 50 commits into
base: master
Choose a base branch
from

Conversation

marcusbfs
Copy link
Collaborator

No description provided.

@marcusbfs marcusbfs marked this pull request as ready for review November 20, 2024 13:12
@marcusbfs marcusbfs requested a review from itsfarseen November 20, 2024 13:12
For some reason, CSL serializes `ScriptPubkey` as a `NativeScript`
instead.

`NativeScript` is a tagged record, of which the first variant (variant
tag `0`) is of type `ScriptPubkey`. CDL represents a `ScriptPubkey` as
a newtype wrapper over a Ed25519KeyHash, so it simply serializes the hash
whenever the `ScriptPubKey` needs to be serialized. But CSL instead
serializes a `ScriptPubKey` by first serializing the tag `0` followed by
the actual hash (so, in reality, it is serializing the parent component
`NativeScript`).

The truth is that there is little reason to serialize a `ScriptPubKey`
alone, because this type is only used as a variant of `NativeScript`. So
this choice has no consequence when serializing/deserializing entire
transactions (both CSL and CDL know how to leverage the
serialize/deserialize methods of `ScriptPubKey` to serialize/deserialize
a `NativeScript` correctly). But this difference in implementation does
trigger errors in our suite, since we test each separate component,
including each branch of every sum type.

For the moment I just blacklisted this variant so it's not added to the
test tree, but this behaviour might happen with other (if not all) tagged
records that use variants with types that are never used alone. If that
is the case, we might need to change how we serialize tagged records and
its variants to match CSL's behaviour.
@marcusbfs marcusbfs force-pushed the marcusbfs/missing-functions branch from 80081e3 to fb89ca9 Compare December 4, 2024 23:36
@itsfarseen
Copy link
Collaborator

@marcusbfs once #26 is merged, could you rebase on master and commit the output of the codegen?
Right now the CI is failing due to the generated files being out of sync.

@marcusbfs marcusbfs force-pushed the marcusbfs/missing-functions branch from 0d76746 to d74b0dd Compare December 9, 2024 19:06
@marcusbfs marcusbfs force-pushed the marcusbfs/missing-functions branch from d74b0dd to 1564744 Compare December 12, 2024 10:26
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.

3 participants