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

types: Move complexity check to (SpendPolicy).Verify #236

Merged
merged 2 commits into from
Nov 27, 2024

Conversation

lukechampine
Copy link
Member

Same rationale as #234: this logic wasn't present in UnmarshalJSON, leading to inconsistency.

I also tweaked the behavior of thresh. thresh is supposed to require exactly n policies to be satisfied (so I suppose "threshold" is a bit of a misnomer), so opaque is used to hide unsatisfied policies. But Verify was only checking that pk and h policies were replaced with opaque; above and after were allowed to fail without causing the entire policy to fail. This isn't necessarily wrong -- and in fact, it's slightly more space-efficient -- but it feels inconsistent, and it's more ambiguous. I like how requiring opaque forces you to be explicit about which sub-policies you're using (and also makes it obvious to the reader which sub-policies are being satisfied).

types/policy.go Show resolved Hide resolved
types/policy_test.go Outdated Show resolved Hide resolved
@n8maninger n8maninger merged commit d3d9f55 into master Nov 27, 2024
9 checks passed
@n8maninger n8maninger deleted the large-policies branch November 27, 2024 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants