-
Notifications
You must be signed in to change notification settings - Fork 1.7k
refactor: Related #9459 - evmbin: replace untyped json! macro with fully typed serde serialization using Rust structs #10657
Conversation
…ad of hardcoding with JSON
It looks like this contributor signed our Contributor License Agreement. 👍 Many thanks, Parity Technologies CLA Bot |
@ltfschoen I think the #9459 was already resolved by #9489, but adding more docs is welcome. |
Closing as this task has already been completed! |
@ordian i still think some of this PR can be useful -- replacing macro with fully typed serialization is a little bit better IMO. My opinion is that, not only we may be using it somewhere else in the future, but also, it allows us to customize how each field are encoded/decoded (camel case!) so it's a little bit more rusty. :) WDYT? |
@sorpaas I agree the refactoring to use rust structs is better than untyped |
Co-Authored-By: Andronik Ordian <write@reusable.software>
…_gas success Co-Authored-By: Andronik Ordian <write@reusable.software>
…_gas failure Co-Authored-By: Andronik Ordian <write@reusable.software>
… finish for state root Co-Authored-By: Andronik Ordian <write@reusable.software>
… before_test Co-Authored-By: Andronik Ordian <write@reusable.software>
…r state root Co-Authored-By: Andronik Ordian <write@reusable.software>
…r finish success Co-Authored-By: Andronik Ordian <write@reusable.software>
…r finish failure Co-Authored-By: Andronik Ordian <write@reusable.software>
@dvdplm could you take a look again? |
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.
minor grumble
Co-Authored-By: David <dvdplm@gmail.com>
…me-parent * master: refactor: whisper: Add type aliases and update rustdocs in message.rs (#10812) Break circular dependency between Client and Engine (part 1) (#10833) tests: Relates to #10655: Test instructions for Readme (#10835) refactor: Related #9459 - evmbin: replace untyped json! macro with fully typed serde serialization using Rust structs (#10657) idiomatic changes to PodState (#10834) Allow --nat extip:your.host.here.org (#10830) When updating the client or when called from RPC, sleep should mean sleep (#10814) Remove excessive warning (#10831) Fix typo in README.md (#10828) ethcore does not use byteorder (#10829) Better logging when backfilling ancient blocks fail (#10796) depends: Update wordlist to v1.3 (#10823)
* master: refactor: whisper: Add type aliases and update rustdocs in message.rs (#10812) Break circular dependency between Client and Engine (part 1) (#10833) tests: Relates to #10655: Test instructions for Readme (#10835) refactor: Related #9459 - evmbin: replace untyped json! macro with fully typed serde serialization using Rust structs (#10657) idiomatic changes to PodState (#10834) Allow --nat extip:your.host.here.org (#10830) When updating the client or when called from RPC, sleep should mean sleep (#10814) Remove excessive warning (#10831) Fix typo in README.md (#10828) ethcore does not use byteorder (#10829) Better logging when backfilling ancient blocks fail (#10796) depends: Update wordlist to v1.3 (#10823) cargo update -p smallvec (#10822) replace memzero with zeroize crate (#10816) Don't repeat the logic from Default impl (#10813) removed additional_params method (#10818) Add Constantinople eips to the dev (instant_seal) config (#10809)
* master: (21 commits) refactor: whisper: Add type aliases and update rustdocs in message.rs (#10812) Break circular dependency between Client and Engine (part 1) (#10833) tests: Relates to #10655: Test instructions for Readme (#10835) refactor: Related #9459 - evmbin: replace untyped json! macro with fully typed serde serialization using Rust structs (#10657) idiomatic changes to PodState (#10834) Allow --nat extip:your.host.here.org (#10830) When updating the client or when called from RPC, sleep should mean sleep (#10814) Remove excessive warning (#10831) Fix typo in README.md (#10828) ethcore does not use byteorder (#10829) Better logging when backfilling ancient blocks fail (#10796) depends: Update wordlist to v1.3 (#10823) cargo update -p smallvec (#10822) replace memzero with zeroize crate (#10816) Don't repeat the logic from Default impl (#10813) removed additional_params method (#10818) Add Constantinople eips to the dev (instant_seal) config (#10809) removed redundant fmt::Display implementations (#10806) revert changes to .gitlab-ci.yml (#10807) Add filtering capability to `parity_pendingTransactions` (issue 8269) (#10506) ...
* master: Extricate PodAccount and state Account to own crates (#10838) logs (#10817) refactor: whisper: Add type aliases and update rustdocs in message.rs (#10812) Break circular dependency between Client and Engine (part 1) (#10833) tests: Relates to #10655: Test instructions for Readme (#10835) refactor: Related #9459 - evmbin: replace untyped json! macro with fully typed serde serialization using Rust structs (#10657)
…hore/extricate-state-backend * dp/chore/extricate-account-db-into-own-crate: third time's the charm test failure 2 test failure Extricate PodAccount and state Account to own crates (#10838) logs (#10817) refactor: whisper: Add type aliases and update rustdocs in message.rs (#10812) Break circular dependency between Client and Engine (part 1) (#10833) tests: Relates to #10655: Test instructions for Readme (#10835) refactor: Related #9459 - evmbin: replace untyped json! macro with fully typed serde serialization using Rust structs (#10657)
Purpose: Refactoring to replace untyped json! macro of #9489 with fully typed serde serialization using Rust structs for reusability and allows customization of how each field is encoded/decoded (i.e. camel case)
Fixes #228
orRelated #1337
.multirust
(deprecated) torustup
json!
wherever it is used apart from just TraceDataTraceData
in json.rs and std_json.rs. Status: resolved resolution of theClone
errors.**Question** (solved) in json.rs i replaced use of the untyped json! macro with serde serialization, and i had to use `informant.clone()` here https://github.com//pull/10657/files#diff-98c535329ae4c9199c5b7ddd6b06b5baR85, so i had to add `Clone` to the struct here https://github.com//pull/10657/files#diff-98c535329ae4c9199c5b7ddd6b06b5baR30, and when i ran the tests they passed.
but when i did the same thing of using informant.clone() in the std_json.rs file here https://github.com/paritytech/parity-ethereum/pull/10657/files#diff-7c765744595c52ae7a64296e4264971cR206, and added
Clone
to the associated struct here https://github.com/paritytech/parity-ethereum/pull/10657/files#diff-7c765744595c52ae7a64296e4264971cR55, i get heaps of errors wrong number of type arguments: expected 3, found 2.but when i add the additional first type as either
Clone
orclone::std:clone::Clone
to address all the errors in commit 42ebcf7, when i run the tests again withcargo test -p evmbin --release -- --nocapture
i get a new set of errors, and heaps of them that include: the traitstd::clone::Clone
cannot be made into an object, and doesn't have a size known at compile-time (which i suspect i'd resolve with lifetimes).i could continue trying to fix those errors, but i think i've done something wrong.
stack: informant.clone().stack,
where we clone the entire informant and have to addInformant<Clone
, it could instead beinformant.stack.clone()
, however we can do better by not cloning the stack and storage and rather borrowing, it involves some lifetimes, but doesn't require cloning at all.Clone
Check that it works by copying create2callPrecompiles.json and teststate.json from #10742 and run: