FIP-0086: Merklize The Tipset List #1004
Merged
+203
−9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We use byte, not uint64.
Stebalien
requested review from
momack2,
arajasek,
jennijuju,
kaitlin-beegle,
anorth,
raulk,
jsoares and
TippyFlitsUK
as code owners
jsoares
approved these changes
May 9, 2024
There was a problem hiding this comment.
Editorial review. I don't love going from pseudocode in the rest of the FIP to full go implementation here, but I suppose it's short and clear enough. This all seems to make sense otherwise, but I understand we're waiting on a review from @Kubuxu.
Co-authored-by: Jorge M. Soares <547492+jsoares@users.noreply.github.com>
anorth
reviewed
May 9, 2024
There was a problem hiding this comment.
I would also somewhat prefer the FIP document itself stuck to pseudocode. But since merkle trees are a fairly widely understood concept, how about pulling this code out into a supporting file under resources and pointing to it there?
Regarding decision about ECTipset, I think the conclusions stated in filecoin-project/go-f3#166 are strong and we should probably go with that.
Can do. Although it's surprisingly easy to mess up the implementation (or design...).
This change so-far handles point's 1 & 2 (small finality certificate, no need to parse CBOR in a snark) but it doesn't handle point 3. IMO, that was the weakest point... but it's also not particularly hard to address it (it just adds a small bit of complexity). |
Kubuxu
reviewed
May 10, 2024
Kubuxu
reviewed
May 10, 2024
Unfortunately, this means that F3 will need to start _caring_ about the contents of these tipsets again. But that's unavoidable if we want to sign a different format than the one we send over the wire.
jennijuju
approved these changes
May 13, 2024
|
editor review ✅ . @Kubuxu can you confirm this #1004 (comment) and see if the flag is resolved? will block merging the PR until then |
Stebalien
added a commit
to filecoin-project/go-f3
that referenced
this pull request
May 14, 2024
This patch implements the changes described in filecoin-project/FIPs#1004. Unfortunately, go-f3 now needs to be aware of the internal structure of `TipSet` objects as it sends them over the wire in one format, but signs another. This effectively reverts #149. Specific changes: 1. On the wire (and in finality certificates), `TipSet` objects are now directly cbor-marshaled within the payload instead of first being marshaled to byte strings. 2. Instead of directly covering the entire tipset list with the payload signature: 1. `TipSet` objects are "marshaled for signing" per the FIP change proposed above. 2. These marshaled tipset objects are stuffed into a merkle tree. 3. The merkle tree root is signed (along with the payload's other fields), again according the proposed changes. fixes #166
Stebalien
added a commit
to filecoin-project/go-f3
that referenced
this pull request
May 14, 2024
This patch implements the changes described in filecoin-project/FIPs#1004. Unfortunately, go-f3 now needs to be aware of the internal structure of `TipSet` objects as it sends them over the wire in one format, but signs another. This effectively reverts #149. Specific changes: 1. On the wire (and in finality certificates), `TipSet` objects are now directly cbor-marshaled within the payload instead of first being marshaled to byte strings. 2. Instead of directly covering the entire tipset list with the payload signature: 1. `TipSet` objects are "marshaled for signing" per the FIP change proposed above. 2. These marshaled tipset objects are stuffed into a merkle tree. 3. The merkle tree root is signed (along with the payload's other fields), again according the proposed changes. fixes #166
Stebalien
added a commit
to filecoin-project/go-f3
that referenced
this pull request
May 14, 2024
This patch implements the changes described in filecoin-project/FIPs#1004. Unfortunately, go-f3 now needs to be aware of the internal structure of `TipSet` objects as it sends them over the wire in one format, but signs another. This effectively reverts #149. Specific changes: 1. On the wire (and in finality certificates), `TipSet` objects are now directly cbor-marshaled within the payload instead of first being marshaled to byte strings. 2. Instead of directly covering the entire tipset list with the payload signature: 1. `TipSet` objects are "marshaled for signing" per the FIP change proposed above. 2. These marshaled tipset objects are stuffed into a merkle tree. 3. The merkle tree root is signed (along with the payload's other fields), again according the proposed changes. fixes #166
Stebalien
added a commit
to filecoin-project/go-f3
that referenced
this pull request
May 14, 2024
This patch implements the changes described in filecoin-project/FIPs#1004. Unfortunately, go-f3 now needs to be aware of the internal structure of `TipSet` objects as it sends them over the wire in one format, but signs another. This effectively reverts #149. Specific changes: 1. On the wire (and in finality certificates), `TipSet` objects are now directly cbor-marshaled within the payload instead of first being marshaled to byte strings. 2. Instead of directly covering the entire tipset list with the payload signature: 1. `TipSet` objects are "marshaled for signing" per the FIP change proposed above. 2. These marshaled tipset objects are stuffed into a merkle tree. 3. The merkle tree root is signed (along with the payload's other fields), again according the proposed changes. fixes #166
Stebalien
added a commit
to filecoin-project/go-f3
that referenced
this pull request
May 14, 2024
This patch implements the changes described in filecoin-project/FIPs#1004. Unfortunately, go-f3 now needs to be aware of the internal structure of `TipSet` objects as it sends them over the wire in one format, but signs another. This effectively reverts #149. Specific changes: 1. On the wire (and in finality certificates), `TipSet` objects are now directly cbor-marshaled within the payload instead of first being marshaled to byte strings. 2. Instead of directly covering the entire tipset list with the payload signature: 1. `TipSet` objects are "marshaled for signing" per the FIP change proposed above. 2. These marshaled tipset objects are stuffed into a merkle tree. 3. The merkle tree root is signed (along with the payload's other fields), again according the proposed changes. fixes #166
It should not block, as it is only a note in the FIP. |
|
@jennijuju we will discuss it but it should not block merge of this PR |
anorth
reviewed
May 14, 2024
| 1. The `Epoch`, encoded as a big-endian 64bit value (8 bytes). | ||
| 2. The `Commitements` hash as-is (32 bytes). | ||
| 3. The tipset _CID_, a blake2b CID of the `TipsetKey` encoded as a CBOR byte array (38 bytes). The resulting CID is the "tipset CID". Importantly, it's hash digest is the digest returned by the `BLOCKHASH` instruction in Filecoin's EVM implementation. | ||
| 4. The `PowerTable` CID . |
The phase is 0x5 (decide) not 0.
Stebalien
force-pushed
the
steb/fip-0086-merkle-tree
branch
from
b98ae5f
to
05d2469
Compare
github-merge-queue bot
pushed a commit
to filecoin-project/go-f3
that referenced
this pull request
May 17, 2024
* EVM-friendly TipSet and ECChain formats This patch implements the changes described in filecoin-project/FIPs#1004. Unfortunately, go-f3 now needs to be aware of the internal structure of `TipSet` objects as it sends them over the wire in one format, but signs another. This effectively reverts #149. Specific changes: 1. On the wire (and in finality certificates), `TipSet` objects are now directly cbor-marshaled within the payload instead of first being marshaled to byte strings. 2. Instead of directly covering the entire tipset list with the payload signature: 1. `TipSet` objects are "marshaled for signing" per the FIP change proposed above. 2. These marshaled tipset objects are stuffed into a merkle tree. 3. The merkle tree root is signed (along with the payload's other fields), again according the proposed changes. fixes #166 * test marshal payload for signing * slightly optimize hashing * add a worst-case benchmark for marshalling payloads * address feedback * Fake marshaling for testing * pre-allocate when marshaling tipsets * combine signer/verifier and MarshalPayloadForSignature Into a single Signatures interface. * tipset marshal for signing test * Test bad merkle-tree paths
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR turns the tipset list into a merkle-tree to shrink the size of the root "decision" message to something more friendly to zkSNARKs and bandwidth-limited blockchains. I've also re-arranged the signature format a bit to align the fields on 32-byte word boundaries to make it a little more EVM friendly.
Second, this PR reformats the
ECTipsetstructs themselves to be more EVM friendly. Specifically:ECTIpsetby concatenating the four fields (epoch, commitments, tipset cid, powertable cid) instead of CBOR-encoding.ECTipsetfor signing, so we can avoid parsing. We likely don't care about the tipset CID and/or the power table CID in bridge smart contracts anyways.See the rational for details.