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

Extra HashableScriptData in TxBody for PK inputs and reference inputs with hashed datums. #637

Closed
wants to merge 2 commits into from

Conversation

mmontin
Copy link

@mmontin mmontin commented Sep 13, 2024

This PR aims to solve issue #600 by allowing users to provide instances of HashableScriptData for hashed datums located:

  • in reference inputs
  • in regular inputs owned by private keys

For now, these data can only be provided:

  • in outputs
  • in regular inputs owned by scripts

This contribution intends to be as little invasive as possible, and to respect the current way of handling the creation of the data map provided in the script context. The map is currently built by browsing through the relevant parts of the body content and retrieving the relevant data there. Thus, the idea here is not to create a new field in the body content, but instead to allow user to add additional HashableScriptData at two addition locations in the body content.

Comment on lines +727 to +730
data PrivateKeyWitness witctx era where
PrivateKeyWitness
:: KeyDatum witctx
-> PrivateKeyWitness witctx era
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this not a GADT? I know there are lots of them floating around but we are actually moving away from using GADTs when not needed.

Copy link
Author

Choose a reason for hiding this comment

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

Yes i can. I basically just mimicked how other types around were defined. But indeed GADTs are not necessary there.

Comment on lines +787 to +789
data KeyDatum witctx where
KeyDatumForTxIn :: Maybe HashableScriptData -> KeyDatum WitCtxTxIn
NoKeyDatumForStake :: KeyDatum WitCtxStake
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this not a GADT? I know there are lots of them floating around but we are actually moving away from using GADTs when not needed.

Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Sorry this is not the cleanest approach to add this functionality. Myself and my team will handle this. Thank you for the attempt!

@mmontin
Copy link
Author

mmontin commented Sep 14, 2024

@Jimbo4350 I barely started and I was in need for feedback in case there was a better way to approach this, but that's fine by me if you handle it.

@mmontin mmontin closed this Sep 14, 2024
@Jimbo4350
Copy link
Contributor

@mmontin #640

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