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
Make Decode canonicalize CBOR. #17
Conversation
if !nd2.Cid().Equals(nd1.Cid()) || !bytes.Equal(nd2.RawData(), nd1.RawData()) { | ||
t.Fatal("re-decoding a canonical node should be idempotent") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If test_objects/non-canon.cbor is taken from go-ipfs I'd add a check for whether CID is zdpuAmxF8q6iTUtkB3xtEYzmc5Sw762qwQJftt5iW8NTWLtjC
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
node.go
Outdated
@@ -77,6 +63,19 @@ func DecodeInto(b []byte, v interface{}) error { | |||
|
|||
} | |||
|
|||
// Decode's a cbor node into an object. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decodes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚶♂️ 🎶
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after @magik6k's comments are addressed
1 similar comment
Also, DRY up CBOR decoding code.
We pre-computed it in e0e7215 but never used it.
1 similar comment
@whyrusleeping it should all be fixed (I've also added a bit of documentation to the Decode/DecodeBlock functions). In the future, do you prefer keeping history intact while a PR is under review? |
Updated ipfs/kubo#4023 with current gx version of this to see if tests pass |
Also:
gx import
go-block-format because I apparently forgot to do this.gx release 1.2.4
so we can use this in ipfs/go-ipfs/#4023I've also imported a variant of the canonicalization test so I don't re-introduce this bug in the future...