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

FMD Parameter Selection Breaking Changes #4137

Merged
merged 15 commits into from
May 21, 2024
Merged

Conversation

cronokirby
Copy link
Contributor

@cronokirby cronokirby commented Mar 29, 2024

The first part of #1087.

This restructures the way FMD parameters are handled so that we can choose different algorithms to update the parameters.

A migration has been written so that we move from the old format, where we had a hard-coded parameter, to a new format, where we have an algorithm as a chain parameter, and then update the current FMD parameter over time based on that algorithm. The protobufs are also structured so that we can easily add new algorithms over time, without making a breaking state change.

By default, the algorithm just chooses a constant parameter, so dynamic FMD params are not part of this PR.

@cratelyn cratelyn added this to the Sprint 3 milestone Apr 1, 2024
@cratelyn cratelyn added the A-shielded-crypto Area: Cryptographic design for Penumbra's shielded transaction model label Apr 1, 2024
@cronokirby cronokirby force-pushed the 1087-fmd-parameter-selection branch from 8738ac9 to ae1e92f Compare April 1, 2024 22:40
@cronokirby cronokirby force-pushed the 1087-fmd-parameter-selection branch from ae1e92f to d4913fa Compare April 2, 2024 01:50
@cronokirby cronokirby force-pushed the 1087-fmd-parameter-selection branch from d4913fa to f50757e Compare April 2, 2024 01:51
@cratelyn cratelyn modified the milestones: Sprint 3, Sprint 4 Apr 8, 2024
@cronokirby cronokirby force-pushed the 1087-fmd-parameter-selection branch from beed4f0 to bf45f7a Compare April 12, 2024 20:04
@cronokirby cronokirby force-pushed the 1087-fmd-parameter-selection branch from bf45f7a to 447b75e Compare April 12, 2024 21:49
@cronokirby cronokirby force-pushed the 1087-fmd-parameter-selection branch 2 times, most recently from 5892f6e to 0e1425c Compare April 18, 2024 05:28
@cratelyn cratelyn modified the milestones: Sprint 4, Sprint 5 Apr 22, 2024
@aubrika aubrika added the friction something made this fall into the following milestone & the reason should be noted in a comment label May 6, 2024
@aubrika aubrika removed this from the Sprint 5 milestone May 6, 2024
@cronokirby cronokirby force-pushed the 1087-fmd-parameter-selection branch from 8ce6712 to e8d64c9 Compare May 21, 2024 16:47
@erwanor erwanor added consensus-breaking breaking change to execution of on-chain data migration A pull request that contains a migration labels May 21, 2024
@cronokirby cronokirby force-pushed the 1087-fmd-parameter-selection branch from e8d64c9 to 3f933c1 Compare May 21, 2024 18:25
@cronokirby cronokirby merged commit b5d13e9 into main May 21, 2024
13 checks passed
@cronokirby cronokirby deleted the 1087-fmd-parameter-selection branch May 21, 2024 19:13
@@ -95,16 +112,20 @@ pub async fn migrate(
tracing::info!(?post_upgrade_root_hash, "post-upgrade root hash");

(
start_time.elapsed().expect("start time is set"),
start_time.elapsed().expect("start time not set"),
Copy link
Member

Choose a reason for hiding this comment

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

@cronokirby fwiw, the convention is that you write why you expect the Result/Option to be Ok/Some

@cronokirby cronokirby restored the 1087-fmd-parameter-selection branch May 24, 2024 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-shielded-crypto Area: Cryptographic design for Penumbra's shielded transaction model consensus-breaking breaking change to execution of on-chain data migration A pull request that contains a migration state-breaking breaking change to on-chain data
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants