Workspaces split #153

rooooooooob · 2022-11-30T02:11:58Z

Splits the existing rust/core and rust/wasm into multiple crates at a root workspace. This also adds in the cip25-lib repo.

Existed before at https://github.com/dcSpark/cip25-lib

Going on the previous two commits bringing in exisitng code over to the workspace structure, this makes it work together using each other as dependencies and such + misc fixes so it compiles. This includes the CIP25 crate as well.

rooooooooob · 2022-11-30T17:01:57Z

Cargo.toml

+
+# this is for the new crate structure. The legacy code (current CML) still resides in the `rust` directory.
+members = [
+    "core",


We'll still need to figure out what to do with the JSON schema generating stuff as we currently have multiple in the project now with CIP25.

could you share a little more context please? what's the problem? seems like I don't have full picture

For the WASM build in legacy CML we have as part of a build script a rust program in rust/json-gen that outputs JSON schemas that define the JSON format of on-chain data. We have a similar script in json-gen-split. We also have that with CIP-25 as well since we used the JSON functionality of cddl-codegen too. The program outputs those schemas then the rest of the build scripts piece those together (see the scripts/ folder + package.json for how the scripts/programs are composed) into the final output .ts file output by wasm-bindgen / wasm-pack to represent the types from JS/typescript. We're going to have to update the build scripts and such now that we'll have multiple wasm builds as well.

chain/Cargo.toml

rooooooooob · 2022-11-30T17:09:50Z

core/src/serialization.rs

+
+    /// Bytes of a structure using the CBOR bytes as per the CDDL spec
+    /// which for foo = bytes will include the CBOR bytes type/len, etc.
+    fn to_cbor_bytes(&self, force_canonical: bool) -> Vec<u8> {


One thing we have different is that CIP25 was generated without encoding preservation so there's this force_canonical concept on serialization. It's ignored by the auto-implementation of Serialize if cbor_event::Serialize is implemented, which is the trait that CIP25 uses. Is that the behavior we wanted? CIP25 was generated without the encoding preservation since it didn't really need it, from what I thought you wouldn't be storing the CBOR here and you'd be using it more for parsing existing metadata. The force_canonical is there since it's used for hardware wallets for encoding tx bodies but might not be too useful in other places.

if it was generated with the encoding preservation, would this return just the original rep? canonical means https://www.rfc-editor.org/rfc/rfc7049#section-3.9 canonical cbor from the standard, right?

So the stuff in chain right now if you pass in false will return whatever format was used when it was deserialized, or canonical if there was no format (e.g. was created from code right now not deserialized). If you pass in true it will turn it into canonical CBOR regardless of how it was created.

In CIP25 whatever you pass in it'll end up being canonical (afaik). So if you deserialize something it won't preserve the format, which was fine before when it was on its own since you're not going to be storing anything here, as this is more just for parsing without reencoding ever.

We could regenerate it with preserve-encdoings=true but that won't work with how it's used in Cardano as the CDDL file for the spec is actually a subset of the input bytes since it only parses the subset of a given tx metadata that conforms to the spec. But to make round-tripping consistent you'd need to figure out a way to remember the encodings of all other parts of the metadata NOT involved in the cip25 spec. We could potentially look at modifying cddl-codegen to figure out a way to do that but I'm not sure it's worth the time or complexity it adds to the crate. It would be nice if it was preserve-encodings for consistency now that we have a mono-repo and it's a part of CML, but on the other hand we could just document that it's not for round-tripping unless we think this is something important. @SebastienGllmt

Yeah, handling this kind of subset operation is a ticket we have no plans to address at the moment

So I was thinking about two options then:
a) Leave it how it is and document that the cip-25 crate doesn't preserve encodings and should not be used for round-tripping.
b) get rid of Serialize::to_cbor_bytes(force_canonical: bool) and instead make 2 other traits SerializeOriginal::to_original_cbor_cbytes() and SerializeCanonical::to_canonical_cbor_bytes() but it's feeling awfully pedantic now. In this case cip-25 would only implement the canonical one and chain would implement both. Although this doesn't explicitly cover the fact that cip-25 ignores fields not part of the spec either.

or something along those lines

cip25-wasm/src/lib.rs

gostkin · 2022-12-01T20:53:55Z

cargo test fails if we run it from workspace folder

cannot find attribute `quickcheck` in this scope
   --> crypto/src/chain_crypto/algorithms/ed25519_derive.rs:116:7
    |
116 |     #[quickcheck]

gostkin · 2022-12-01T20:55:48Z

let's merge #150 , so we can see how the things are with checks before review, @rooooooooob ?

All things specified in babbage/crypto.cddl are now back in the chain crate. To work around some things e.g. signatures, hashes being uses in the crypto crate we have those being their base locations with chain/crypto having a `ChainCrypto<T>` wrapper around this. We have the to/from raw bytes in a `RawBytesEncoding` trait to help out as well for when things are raw bytes. All crypto bindings are still in the `wasm` crate. We will need to look into a separate `crypto-wasm` crate and first test out how those would interact and if it's possible/a good idea.

rooooooooob · 2022-12-02T06:56:41Z

I pushed another commit that moves the on-chain crypto primitives into the chain crate in a crypto module. I kept base (not caring about encodings) versions of these in the crypto crate with ChainCrypto<T> wrappers around these in the chain/crypto module that take care of the encoding information relevant to their on-chain-ness.

rooooooooob · 2022-12-02T07:03:18Z

chain/src/crypto.rs

+        let s = <String as serde::de::Deserialize>::deserialize(deserializer)?;
+        T::from_raw_hex(&s)
+            .map(Into::into)
+            .map_err(|_e| serde::de::Error::invalid_value(serde::de::Unexpected::Str(&s), &"hex bytes for signature"))


Oops copy-paste error. I'll change this message so it doesn't say signature (since this could be a hash too).

gostkin · 2022-12-02T14:18:34Z

chain/src/lib.rs

@@ -200,9 +188,9 @@ pub mod certs;
 pub use certs::*;


-pub mod crypto;
+//pub mod crypto;


can be removed , we have public exposure above

gostkin · 2022-12-02T14:23:27Z

looks good

Crypto wasm bindings moved into separate `crypto-wasm` crate. The on-chain representations are still in `wasm`.

gostkin · 2022-12-07T16:25:51Z

Cargo.toml

+]
+
+# exclude old crate structure to avoid error in it
+exclude = [


how the pipeline will be working? will we have one for old and one for new structure? we need to verify both old and new crates are compiling and working correctly

rooooooooob added 3 commits November 28, 2022 17:37

Workspace refactor initial moving existing code

0fb2027

Move CIP25 library into CML.

0e8b6d6

Existed before at https://github.com/dcSpark/cip25-lib

Base code changes for module split

f9d9052

Going on the previous two commits bringing in exisitng code over to the workspace structure, this makes it work together using each other as dependencies and such + misc fixes so it compiles. This includes the CIP25 crate as well.

rooooooooob added the Rust/WASM Refactor Relating to the refactor from a single crate to two separate crates (rust-only + wasm bindings). label Nov 30, 2022

rooooooooob added this to the Dedicated native rust crate / wasm bindings refactor milestone Nov 30, 2022

rooooooooob commented Nov 30, 2022

View reviewed changes

chain/Cargo.toml Show resolved Hide resolved

rooooooooob commented Nov 30, 2022

View reviewed changes

gostkin reviewed Dec 1, 2022

View reviewed changes

cip25-wasm/src/lib.rs Show resolved Hide resolved

rooooooooob commented Dec 2, 2022

View reviewed changes

gostkin reviewed Dec 2, 2022

View reviewed changes

crypto-wasm crate added

5989c7f

Crypto wasm bindings moved into separate `crypto-wasm` crate. The on-chain representations are still in `wasm`.

gostkin reviewed Dec 7, 2022

View reviewed changes

Merge branch 'develop' into workspaces-split

07b4c9c

gostkin approved these changes Dec 14, 2022

View reviewed changes

gostkin merged commit 1461038 into develop Dec 14, 2022

SebastienGllmt deleted the workspaces-split branch December 15, 2022 01:12

rooooooooob mentioned this pull request Aug 24, 2023

Refactor old structs over to split architecture #138

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Workspaces split #153

Workspaces split #153

rooooooooob commented Nov 30, 2022

rooooooooob Nov 30, 2022

gostkin Dec 1, 2022

rooooooooob Dec 1, 2022

rooooooooob Nov 30, 2022

gostkin Dec 1, 2022

rooooooooob Dec 2, 2022

SebastienGllmt Dec 2, 2022

rooooooooob Dec 5, 2022

gostkin commented Dec 1, 2022 •

edited

gostkin commented Dec 1, 2022

rooooooooob commented Dec 2, 2022

rooooooooob Dec 2, 2022

gostkin Dec 2, 2022

gostkin commented Dec 2, 2022

gostkin Dec 7, 2022

Workspaces split #153

Workspaces split #153

Conversation

rooooooooob commented Nov 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gostkin commented Dec 1, 2022 • edited

gostkin commented Dec 1, 2022

rooooooooob commented Dec 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gostkin commented Dec 2, 2022

Choose a reason for hiding this comment

gostkin commented Dec 1, 2022 •

edited