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

funding: implement execute ActionLiquidityTournamentVote #5033

Merged
merged 4 commits into from
Jan 31, 2025

Conversation

cronokirby
Copy link
Contributor

@cronokirby cronokirby commented Jan 30, 2025

Closes #5032.

The check and execute logic for the action handler is still missing the nullifier check, but there's an obvious insertion point for adding that logic.

Testing deferred.

@cronokirby cronokirby force-pushed the cronokirby/5032-tally branch from 472487a to f57279b Compare January 30, 2025 21:13
@cronokirby cronokirby force-pushed the cronokirby/5032-tally branch from f57279b to a97a3f0 Compare January 30, 2025 22:18
@cronokirby cronokirby marked this pull request as ready for review January 30, 2025 22:18
@cronokirby cronokirby added the consensus-breaking breaking change to execution of on-chain data label Jan 30, 2025
@cronokirby cronokirby force-pushed the cronokirby/5032-tally branch from a97a3f0 to 67a22f7 Compare January 30, 2025 22:28
@cronokirby cronokirby changed the title WIP: #5032 Implement check and execute for LQT votes Jan 31, 2025
self.body.nullifier,
current_epoch.index
);
state.put_lqt_spent_nullifier(current_epoch.index, nullifier, TransactionId([0u8; 32]));
Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think we can revisit in a follow-up PR.

bytes_asset.copy_from_slice(&asset.to_bytes());
bytes_power.copy_from_slice(&((!power).to_be_bytes()));
bytes_voter.copy_from_slice(&voter.to_vec());

Copy link
Member

Choose a reason for hiding this comment

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

👍

/// This will return None if the denom is not a base denom.
pub fn incentivized_id(&self) -> Option<asset::Id> {
REGISTRY
.parse_denom(&self.incentivized.denom)
Copy link
Member

Choose a reason for hiding this comment

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

I presume this still work for unknown denoms right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://rustdoc.penumbra.zone/main/src/penumbra_sdk_asset/asset/registry.rs.html#70

Yeah, the case in which it returns None is only when it's a known non-base denom.

Copy link
Member

@erwanor erwanor left a comment

Choose a reason for hiding this comment

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

ACK writing a zero-ed TxId in the NF set is fine, we can follow up with another PR

@cronokirby cronokirby merged commit f8dcd60 into protocol/lqt_branch Jan 31, 2025
13 checks passed
@cronokirby cronokirby deleted the cronokirby/5032-tally branch January 31, 2025 18:33
@erwanor erwanor mentioned this pull request Jan 31, 2025
28 tasks
@erwanor erwanor changed the title Implement check and execute for LQT votes funding: implement execute ActionLiquidityTournamentVote Jan 31, 2025
conorsch pushed a commit that referenced this pull request Jan 31, 2025
Closes #5032.

The check and execute logic for the action handler is still missing the
nullifier check, but there's an obvious insertion point for adding that
logic.

Testing deferred.
conorsch pushed a commit that referenced this pull request Feb 4, 2025
Closes #5032.

The check and execute logic for the action handler is still missing the
nullifier check, but there's an obvious insertion point for adding that
logic.

Testing deferred.
conorsch pushed a commit that referenced this pull request Feb 5, 2025
Closes #5032.

The check and execute logic for the action handler is still missing the
nullifier check, but there's an obvious insertion point for adding that
logic.

Testing deferred.
conorsch pushed a commit that referenced this pull request Feb 5, 2025
Closes #5032.

The check and execute logic for the action handler is still missing the
nullifier check, but there's an obvious insertion point for adding that
logic.

Testing deferred.
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