dag-pb: further clarifications on link sorting #233
Merged
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
willscott
approved these changes
Aug 2, 2022
RangerMauve
approved these changes
Aug 3, 2022
|
FYI I'm sitting on this one for a moment, it turns out to be much more subtle than saying "before version X" and "after version X" of go-merkledag. There's current behaviours there that still need to be flagged (and probably fixed) so I should capture that accurately. |
rvagg
added a commit
to ipfs/go-merkledag
that referenced
this pull request
Aug 16, 2022
When decoding a badly serialised block with Links out of order, don't sort the list until we receive an explicit mutation operation. This ensures stable DAG traversal ordering based on the links as they appear in the serialised form and removes surprise-sorting when performing certain operations that wouldn't be expected to mutate. The pre-v0.4.0 behaviour was to always sort, but this behaviour wasn't baked in to the dag-pb spec and wasn't built into go-codec-dagpb which now forms the backend of ProtoNode, although remnants of sorting remain in some operations. Almost all CAR-from-DAG creation code in Go uses go-codec-dagpb and go-ipld-prime's traversal engine. However this can result in a different order when encountering badly encoded blocks (unsorted Links) where certain intermediate operations are performed on the ProtoNode prior to obtaining the Links() list (Links() itself doesn't sort, but e.g. RawData() does). The included "TestLinkSorting/decode" test is the only case that passes without this patch. Ref: ipld/ipld#233 Ref: filecoin-project/boost#673 Ref: filecoin-project/boost#675
rvagg
added a commit
to ipfs/go-merkledag
that referenced
this pull request
Aug 16, 2022
When decoding a badly serialised block with Links out of order, don't sort the list until we receive an explicit mutation operation. This ensures stable DAG traversal ordering based on the links as they appear in the serialised form and removes surprise-sorting when performing certain operations that wouldn't be expected to mutate. The pre-v0.4.0 behaviour was to always sort, but this behaviour wasn't baked in to the dag-pb spec and wasn't built into go-codec-dagpb which now forms the backend of ProtoNode, although remnants of sorting remain in some operations. Almost all CAR-from-DAG creation code in Go uses go-codec-dagpb and go-ipld-prime's traversal engine. However this can result in a different order when encountering badly encoded blocks (unsorted Links) where certain intermediate operations are performed on the ProtoNode prior to obtaining the Links() list (Links() itself doesn't sort, but e.g. RawData() does). The included "TestLinkSorting/decode" test is the only case that passes without this patch. Ref: ipld/ipld#233 Ref: filecoin-project/boost#673 Ref: filecoin-project/boost#675
rvagg
force-pushed
the
rvagg/dagpb-link-sorting
branch
from
2f35a8f
to
06bcd48
Compare
|
I've updated with more detail and referred to an as yet unreleased |
rvagg
added a commit
to ipfs/go-merkledag
that referenced
this pull request
Aug 22, 2022
When decoding a badly serialised block with Links out of order, don't sort the list until we receive an explicit mutation operation. This ensures stable DAG traversal ordering based on the links as they appear in the serialised form and removes surprise-sorting when performing certain operations that wouldn't be expected to mutate. The pre-v0.4.0 behaviour was to always sort, but this behaviour wasn't baked in to the dag-pb spec and wasn't built into go-codec-dagpb which now forms the backend of ProtoNode, although remnants of sorting remain in some operations. Almost all CAR-from-DAG creation code in Go uses go-codec-dagpb and go-ipld-prime's traversal engine. However this can result in a different order when encountering badly encoded blocks (unsorted Links) where certain intermediate operations are performed on the ProtoNode prior to obtaining the Links() list (Links() itself doesn't sort, but e.g. RawData() does). The included "TestLinkSorting/decode" test is the only case that passes without this patch. Ref: ipld/ipld#233 Ref: filecoin-project/boost#673 Ref: filecoin-project/boost#675
rvagg
added a commit
to ipfs/go-merkledag
that referenced
this pull request
Aug 25, 2022
When decoding a badly serialised block with Links out of order, don't sort the list until we receive an explicit mutation operation. This ensures stable DAG traversal ordering based on the links as they appear in the serialised form and removes surprise-sorting when performing certain operations that wouldn't be expected to mutate. The pre-v0.4.0 behaviour was to always sort, but this behaviour wasn't baked in to the dag-pb spec and wasn't built into go-codec-dagpb which now forms the backend of ProtoNode, although remnants of sorting remain in some operations. Almost all CAR-from-DAG creation code in Go uses go-codec-dagpb and go-ipld-prime's traversal engine. However this can result in a different order when encountering badly encoded blocks (unsorted Links) where certain intermediate operations are performed on the ProtoNode prior to obtaining the Links() list (Links() itself doesn't sort, but e.g. RawData() does). The included "TestLinkSorting/decode" test is the only case that passes without this patch. Ref: ipld/ipld#233 Ref: filecoin-project/boost#673 Ref: filecoin-project/boost#675
rvagg
force-pushed
the
rvagg/dagpb-link-sorting
branch
from
acd0ecf
to
1c82a0d
Compare
Jorropo
pushed a commit
to ipfs/boxo
that referenced
this pull request
Mar 15, 2023
When decoding a badly serialised block with Links out of order, don't sort the list until we receive an explicit mutation operation. This ensures stable DAG traversal ordering based on the links as they appear in the serialised form and removes surprise-sorting when performing certain operations that wouldn't be expected to mutate. The pre-v0.4.0 behaviour was to always sort, but this behaviour wasn't baked in to the dag-pb spec and wasn't built into go-codec-dagpb which now forms the backend of ProtoNode, although remnants of sorting remain in some operations. Almost all CAR-from-DAG creation code in Go uses go-codec-dagpb and go-ipld-prime's traversal engine. However this can result in a different order when encountering badly encoded blocks (unsorted Links) where certain intermediate operations are performed on the ProtoNode prior to obtaining the Links() list (Links() itself doesn't sort, but e.g. RawData() does). The included "TestLinkSorting/decode" test is the only case that passes without this patch. Ref: ipld/ipld#233 Ref: filecoin-project/boost#673 Ref: filecoin-project/boost#675 This commit was moved from ipfs/go-merkledag@48c7202
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 is shaping up to be a likely explanation for inconsistencies in Filecoin deals, with traversals yielding differently ordered blocks. I'm writing up more detailed notes on that case for an issue on https://github.com/filecoin-project/boost but the important thing here is to note that there is a difference, and that we've moved almost all of the ecosystem away from sort-on-decode, but that if you want stable traversals then you shouldn't mix and match the two.