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
New metadata format #296
New metadata format #296
Conversation
Codecov Report
@@ Coverage Diff @@
## master #296 +/- ##
=========================================
Coverage ? 72.45%
=========================================
Files ? 76
Lines ? 6604
Branches ? 0
=========================================
Hits ? 4785
Misses ? 1819
Partials ? 0
Continue to review full report at Codecov.
|
Superseedes #264 |
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
I very much like that you actually added a wholesome test.
Let's merge this as soon as Polkadot JS gave their acknowledgement that their JSON parser can trivially parse the output of this PR.
Is it possible to sneak in a version number into the |
I was talking to Sean Young who implements the Solang Solidity to Wasm compiler and we want to share our metadata format for the contracts. Also we though that some other metadata would be nice to have. What do you think? cc @seanyoung |
@Robbepop It makes sense, the additional metadata is quite easy to add as a sub-structure, and is certainly very beneficial as we expand. |
Should this versioning/language field be done as a separate PR? |
Yes, also can you write an issue for that so we can track and discuss there what needs to be added exactly? |
Going through, think this is not correctly named/consistent -
|
So should |
Well |
|
@jacogr are we good to go on your end? |
# Conflicts: # .gitlab-ci.yml # abi/Cargo.toml # core/Cargo.toml # examples/delegator/Cargo.toml # examples/delegator/accumulator/Cargo.toml # examples/delegator/adder/Cargo.toml # examples/delegator/subber/Cargo.toml # examples/dns/Cargo.toml # examples/erc20/Cargo.toml # examples/erc721/Cargo.toml # examples/flipper/Cargo.toml # examples/incrementer/Cargo.toml # examples/multisig_plain/Cargo.toml # examples/runtime-storage/Cargo.toml # lang/macro/Cargo.toml # primitives/Cargo.toml
bors try |
tryBuild failed |
Now updated to use
|
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 but we should merge this after type-info
PR is merged into master
and replace the explicit branches here.
@@ -83,3 +90,15 @@ impl InkProject { | |||
} | |||
} | |||
} | |||
|
|||
fn hex_encode<S>(bytes: &[u8], serializer: S) -> Result<S::Ok, S::Error> |
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.
Could use some documentation with a simple example.
# Conflicts: # primitives/src/key.rs
Codecov Report
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 86.83% 88.11% +1.28%
==========================================
Files 135 136 +1
Lines 5877 5924 +47
==========================================
+ Hits 5103 5220 +117
+ Misses 774 704 -70
Continue to review full report at Codecov.
|
I know the current storage module is being replaced, but I have fixed the compilation errors anyway |
Latest incrementer
|
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
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
|
||
#[test] | ||
fn construct_selector_must_serialize_to_hex() { | ||
// given | ||
let name = "foo"; | ||
let cs: ConstructorSpec<MetaForm> = ConstructorSpec { | ||
name, | ||
selector: 123_456_789u32.to_be_bytes(), | ||
args: Vec::new(), | ||
docs: Vec::new(), | ||
}; | ||
let mut registry = Registry::new(); | ||
|
||
// when | ||
let json = serde_json::to_string(&cs.into_compact(&mut registry)).unwrap(); | ||
|
||
// then | ||
assert_eq!( | ||
json, | ||
r#"{"name":1,"selector":"[\"0x07\",\"0x5B\",\"0xCD\",\"0x15\"]","args":[],"docs":[]}"# | ||
); | ||
} |
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.
Was this test moved or removed?
Closes #294.
"range.offset"
use"range": { "offset" }
camelCase
instead ofsnake_case
"[\"0x07\",\"0x5B\",\"0xCD\",\"0x15\"]"
use"0x075bcd15"
type-metadata
to https://github.com/paritytech/scale-infoThe new type registry serialization depends on paritytech/scale-info#3, so only once that is merged will you get the full new format.
e.g. Incrementer: