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

feat: Add native scripts #241

Merged
merged 3 commits into from
Apr 13, 2022
Merged

feat: Add native scripts #241

merged 3 commits into from
Apr 13, 2022

Conversation

SmaugPool
Copy link
Collaborator

@SmaugPool SmaugPool commented Apr 10, 2022

This is used by https://github.com/SmaugPool/oura-script-sink to make all minting policies available on pool.pm.

I'm not sure if we would like to be able to distinguish scripts coming from witnesses from those coming from auxiliary data.
For now this is not the case.

@SmaugPool SmaugPool changed the title Add native scripts feat: Add native scripts Apr 10, 2022
scarmuega
scarmuega previously approved these changes Apr 10, 2022
@scarmuega
Copy link
Member

@SmaugPool this is great! A feature much overdue.

One question though, any particular reason you're using camelCase for the event record fields? Is it a hard requirement?

We're using snake_case for the rest of the records, would be nice to maintain the same convention across the board.

src/mapper/map.rs Show resolved Hide resolved
src/mapper/map.rs Outdated Show resolved Hide resolved
@@ -228,6 +232,19 @@ impl EventWriter {
self.append(EventData::BlockEnd(record))?;
}

for witness in block.transaction_witness_sets.iter() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we would like to allow to distinguish witnesses scripts from auxiliary data ones?

src/mapper/map.rs Show resolved Hide resolved
src/model.rs Show resolved Hide resolved
@SmaugPool
Copy link
Collaborator Author

SmaugPool commented Apr 10, 2022

@SmaugPool this is great! A feature much overdue.

One question though, any particular reason you're using camelCase for the event record fields? Is it a hard requirement?

We're using snake_case for the rest of the records, would be nice to maintain the same convention across the board.

Yes, this is needed to rebuild the script exactly in its original format, see my comment here:

https://github.com/txpipe/oura/pull/241/files#r846800039

And notice how Cardano SImple scripts use camel case:
https://github.com/input-output-hk/cardano-node/blob/master/doc/reference/simple-scripts.md

Thanks to this serialization format, running cardano-cli transaction policyid on the resulting json returns the correct policy ID.

I don't think it goes against Oura convention, I just consider it as a Cardano object & format and not an Oura one.

@rvcas
Copy link
Member

rvcas commented Apr 10, 2022

I think the camelCase is fine. It's an appropriate usage of serde's rename all

@scarmuega
Copy link
Member

@SmaugPool please flag me when you consider the PR ready for review / merge. LGTM, but I'm not sure if you consider it done.

@SmaugPool
Copy link
Collaborator Author

SmaugPool commented Apr 12, 2022

@SmaugPool please flag me when you consider the PR ready for review / merge. LGTM, but I'm not sure if you consider it done.

@scarmuega It's ready from my point of view.

I will also release version 1.0.0 of my sink as soon as an Oura release includes native scripts.

@scarmuega scarmuega merged commit 30426df into txpipe:main Apr 13, 2022
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