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

Resolve SSW-308 #60

Merged
merged 1 commit into from
Mar 5, 2024
Merged

Resolve SSW-308 #60

merged 1 commit into from
Mar 5, 2024

Conversation

Quantumplation
Copy link
Member

We check that the settings NFT is paid into the settings script with a
valid datum; this isn't strictly neccesary, because we can just say the
protocol is only valid if this happens correctly, but it does help for
an inductive base case.

We check that the settings NFT is paid into the settings script with a
valid datum; this isn't strictly neccesary, because we can just say the
protocol is only valid if this happens correctly, but it does help for
an inductive base case.
Copy link
Collaborator

@francolq francolq left a comment

Choose a reason for hiding this comment

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

Looks good. It solves SSW-308 to a great extend, by checking that the settings UTxO is created with a value that includes the settings NFT token and a datum that is compatible with the SettingsDatum type.

No checks are done on other aspects of the value, so garbage tokens can be included, but we don't consider this an issue.
Also, it could be possible to check for some datum fields, for instance to have correct range of values for base_fee, simple_fee and strategy_fee. I also consider this as completely optional and only if an extremely pedantic validation is wanted.

@Quantumplation
Copy link
Member Author

I'm going to skip tests on this one, as it's largely cosmetic minting checks.

@Quantumplation Quantumplation merged commit f007b79 into main Mar 5, 2024
1 check passed
@Quantumplation Quantumplation deleted the pi/SSW-308-settings-checks branch March 5, 2024 08:29
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.

2 participants