Skip to content

Conversation

@darosior
Copy link
Contributor

@darosior darosior commented Sep 27, 2021

Based on #72 this separates IsSafe and IsSafeTopLevel as suggested in #41 (comment) .

@darosior
Copy link
Contributor Author

Changed now that we decided a Miniscript whose script size is nonstandard would be invalid. Now i'm not sure it really make sense to keep this PR open just for 4d866f1 which could be a small diff in #41.


//! Do all sanity checks.
bool IsSafeTopLevel() const { return GetType() << "Bms"_mst && CheckOpsLimit() && CheckStackSize(); }
bool IsSane() const { return GetType() << "m"_mst && CheckOpsLimit() && CheckStackSize() && IsValid(); }
Copy link
Owner

Choose a reason for hiding this comment

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

Now that there is an IsSane(), perhaps all the ad-hoc sanity checks in the compiler can be replaced with just a call to this? (see Compilation::Add).

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this would be updated with k property too based on what gets merged first

@darosior
Copy link
Contributor Author

Rebased after #41 merge

@sipa
Copy link
Owner

sipa commented Sep 29, 2021

ACK 253172e

@sipa sipa merged commit 385c98e into sipa:master Sep 29, 2021
@darosior darosior deleted the is_safe branch September 29, 2021 16:57
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