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

Implement and update Cbor encoding #157

Merged
merged 22 commits into from
Jan 15, 2020
Merged

Implement and update Cbor encoding #157

merged 22 commits into from
Jan 15, 2020

Conversation

austinabell
Copy link
Contributor

@austinabell austinabell commented Jan 14, 2020

  • Completes Setup and investigate consistent cbor encoding/decoding #35 and refactors previous work done (but keeps the Cbor trait for convenience for encoding structs easier)
    • Note the unmarshall function on the trait was removed, it didn't add much value and required all types which implements the trait specify a lifetime, which gets unnecessarily tedious)
  • Change all references for serde related dependencies, only one that still uses serde is needed for the Serialize and Deserialize derive macro (since the macro puts serde as plain text and can't alias)
  • Sets up IPLD visitor to decode cbor encoded bytes as IPLD (step toward or might be replaced with Review Readiness of Rust IPLD libraries for IPLD Store #111 depending on libraries usage)
  • Changes gas_price and gas_limit to u128 from BigUInt because deriving serialize on that type is a huge mess, but will either create a type for the gas values, or wrap the big ints to more easily define the serialization functions on them and make it more usable (Wrapper for BigInt type #130 BigInt wrapper and local types #135)

Will open this up as draft until I write tests for all of this. I can either open this now and add extensive tests in another PR or just add them and open after. Would love input either way to see what you guys think of this direction (specifically the Cbor trait we have)

@ec2
Copy link
Member

ec2 commented Jan 14, 2020

Big chungus PR... Absolute unit

@austinabell austinabell marked this pull request as ready for review January 14, 2020 19:27
ipld/cid/src/lib.rs Outdated Show resolved Hide resolved
ipld/cid/src/to_cid.rs Outdated Show resolved Hide resolved
@austinabell austinabell requested a review from ec2 January 15, 2020 18:30
Copy link
Contributor

@dutterbutter dutterbutter left a comment

Choose a reason for hiding this comment

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

Great work on this @austinabell 🔥🔥🔥🔥

ipld/cid/src/lib.rs Outdated Show resolved Hide resolved
ipld/cid/src/lib.rs Outdated Show resolved Hide resolved
ipld/cid/src/to_cid.rs Outdated Show resolved Hide resolved
vm/runtime/Cargo.toml Outdated Show resolved Hide resolved
vm/state_tree/Cargo.toml Outdated Show resolved Hide resolved
Copy link
Member

@ansermino ansermino left a comment

Choose a reason for hiding this comment

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

This is missing a lot of docs

@austinabell austinabell merged commit b5295e2 into master Jan 15, 2020
@austinabell austinabell deleted the austin/cbor/impl branch January 15, 2020 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants