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

LQT: fix start position <=> epoch comparison #5071

Merged

Conversation

cronokirby
Copy link
Contributor

Describe your changes

lqt: fix epoch <=> start position comparison

This allows planners to correctly use the first position in the epoch.
This works because the ZK proof verifies that the start position is strictly less.

c.f.

delegator_position_var.position.enforce_cmp(

Testing deferred.

Checklist before requesting a review

  • I have added guiding text to explain how a reviewer should test these changes.

  • If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason:

This allows planners to correctly use the first position in the epoch.
This works because the ZK proof verifies that the start position is strictly less.

c.f. https://github.com/penumbra-zone/penumbra/blob/7869803be0b844f09c8c5efeeaa92384b8a6ba35/crates/core/component/governance/src/delegator_vote/proof.rs#L234
@cronokirby cronokirby added the consensus-breaking breaking change to execution of on-chain data label Feb 7, 2025
@cronokirby
Copy link
Contributor Author

Maybe we should just restrict the check to be for (e, 0, 0) specifically, since that's the only thing I expect planners to produce

@erwanor erwanor merged commit 1d70493 into protocol/lqt_branch Feb 7, 2025
10 checks passed
@erwanor erwanor deleted the cronokirby/lqt-start-position-check-fix branch February 7, 2025 15:55
conorsch pushed a commit that referenced this pull request Feb 14, 2025
## Describe your changes

lqt: fix epoch <=> start position comparison

This allows planners to correctly use the first position in the epoch.
This works because the ZK proof verifies that the start position is
strictly less.

c.f.
https://github.com/penumbra-zone/penumbra/blob/7869803be0b844f09c8c5efeeaa92384b8a6ba35/crates/core/component/governance/src/delegator_vote/proof.rs#L234

Testing deferred.

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus-breaking breaking change to execution of on-chain data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants