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

Commit to a next power table and additional commitments #273

Merged
merged 7 commits into from
May 29, 2024

Conversation

Stebalien
Copy link
Member

@Stebalien Stebalien commented May 22, 2024

In addition to committing to a power table in each tipset, agree on:

  1. The next power table (used to validate the next instance).
  2. Arbitrary commitments (will be used by F3 for snark-friendly power tables, and other fields).

fixes #257

@Stebalien
Copy link
Member Author

Posting this so we can discuss with respect to #151.

gpbft/participant.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented May 22, 2024

Codecov Report

Attention: Patch coverage is 25.84270% with 66 lines in your changes are missing coverage. Please review.

Project coverage is 70.11%. Comparing base (072fd61) to head (707dcab).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #273      +/-   ##
==========================================
- Coverage   71.78%   70.11%   -1.67%     
==========================================
  Files          35       35              
  Lines        2545     2617      +72     
==========================================
+ Hits         1827     1835       +8     
- Misses        588      651      +63     
- Partials      130      131       +1     
Files Coverage Δ
gpbft/chain.go 93.65% <100.00%> (ø)
sim/host.go 94.28% <100.00%> (ø)
sim/signing/fake.go 84.81% <100.00%> (+0.39%) ⬆️
gpbft/participant.go 88.65% <50.00%> (ø)
gpbft/gpbft.go 83.90% <85.00%> (-0.26%) ⬇️
gpbft/gen.go 0.00% <0.00%> (ø)

@Stebalien Stebalien changed the title [WIP] Commit to a base/next power table with additional commitments Commit to a next power table and additional commitments May 24, 2024
Copy link

Fuzz test failed on commit ccce9d6. To troubleshoot locally, download the seed corpus using GitHub CLI by running:

gh run download 9218069385 -n testdata

Aleternatively, download directly from here.

@Stebalien Stebalien force-pushed the steb/root-power-table branch 3 times, most recently from 11ce69c to c46676d Compare May 24, 2024 20:58
gpbft/participant.go Outdated Show resolved Hide resolved
@Stebalien Stebalien marked this pull request as ready for review May 24, 2024 21:00
@Stebalien
Copy link
Member Author

The first commit contains all the changes, the last one just renames Payload.Value to Payload.Chain (which we can drop, land in a different PR, etc. if it becomes a merge issue).

This isn't quite ready for merging yet, but I would like some feedback before I put the finishing touches (tests) on it.

Copy link

Fuzz test failed on commit c46676d. To troubleshoot locally, download the seed corpus using GitHub CLI by running:

gh run download 9229851709 -n testdata

Aleternatively, download directly from here.

@Stebalien Stebalien force-pushed the steb/root-power-table branch 2 times, most recently from dedcb40 to c06302f Compare May 28, 2024 16:37
@Stebalien
Copy link
Member Author

I think we're ready for a final review and merging here unless we want to wait for power table lookback to land first.

@Stebalien Stebalien requested a review from Kubuxu May 28, 2024 16:55
@Stebalien Stebalien force-pushed the steb/root-power-table branch 2 times, most recently from 00bab73 to 458718c Compare May 28, 2024 17:05
@Stebalien
Copy link
Member Author

Ah, actually, let's land #264 today and then I can rebase/finish this one.

gpbft/gpbft.go Outdated Show resolved Hide resolved
gpbft/gpbft.go Outdated Show resolved Hide resolved
gpbft/gpbft.go Outdated Show resolved Hide resolved
gpbft/gpbft.go Outdated Show resolved Hide resolved
gpbft/participant.go Outdated Show resolved Hide resolved
In addition to committing to the power table for each tipset, we commit
to the power table used to validate the next instance in the instance
root (along with a merkle-tree of arbitrary commitments).

fixes #257
That way we match tipsets. We could sign just the payloads in both cases
but... alignment isn't _that_ important here. The only important part is
that the first 3 words (32-byte) are aligned:

1. The first word has to be created inside the smart contract. The
32-byte alignment lets us AND the fixed prefix and the 8-byte instance
number.
2. The second word is the commitment tree root and will likely be used
by, e.g., snarks for validation.
3. The third word is the root of the tipset tree.

But smart contracts aren't usually going to even _look_ at the power
table CID. And if they do, the variable-size is going to be the least of
their costs/complexity.
@Stebalien Stebalien added this pull request to the merge queue May 29, 2024
Merged via the queue into main with commit 2f910b7 May 29, 2024
4 checks passed
@Stebalien Stebalien deleted the steb/root-power-table branch May 29, 2024 17:52
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.

Commit to the next power table in the message payload
4 participants