feat: extend trace decoder err info #148

vladimir-trifonov · 2024-04-05T13:26:58Z

Extend trace decoder err info

Issue: #121

* feat: swap out the internal U512 inside nibbles * fix: comment fix * fix: fix clippy pr issues * fix: fix clippy issue * fix: fix pr comments * docs: update changelog * fix: update impl_to_nibbles --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com>

* MAX fixes for clippy * fix transmut without annotations * please the fmt gods

BGluth

Nice! You did this pretty much exactly the way I was thinking about doing it, and the builder pattern ended up working nicely here.

trace_decoder/src/decoding.rs

BGluth · 2024-04-16T20:19:00Z

trace_decoder/src/utils.rs

-pub(crate) fn write_optional<T: std::fmt::Display>(
-    f: &mut std::fmt::Formatter,
-    label: &str,
-    value: Option<T>,
-) -> std::fmt::Result {
-    if let Some(v) = value {
-        writeln!(f, "{}: {}", label, v)?;
-    }
-    Ok(())
+pub(crate) fn optional_field<T: std::fmt::Debug>(label: &str, value: Option<T>) -> String {
+    value.map_or(String::new(), |v| format!("{}: {:?}\n", label, v))
+}


I actually prefer your original direct call to write! just because it avoids the Vec allocation 😅, but this is totally fine! I know we've already had a back an fourth on this.

Nashtare

LGTM!

muursh · 2024-04-18T13:39:05Z

@vladimir-trifonov can you update the change log and we'll merge? Also are there other PRs of yours we missed from the change log?

vladimir-trifonov · 2024-04-18T15:47:29Z

@vladimir-trifonov can you update the change log and we'll merge? Also are there other PRs of yours we missed from the change log?

one more, but I just added it

* Update plonky2 dependencies (#119) * Update plonky2 dependencies * Modify changelog * Charge gas before SLOAD and refactor `insert_accessed_storage_keys` (#117) * Charge gas before SLOAD and refactor `insert_accessed_storage_keys` * fmt * Only store value for cold access * PR feedback --------- Co-authored-by: BGluth <gluthb@gmail.com> * Increased the public interface for `trie_tools` (#123) - `trie_tools` needs to access a bit of currently private logic. Specifically, it needs to be able to process compact bytecode into `mpt_tries` directly. - I think this change is actually reasonable. I can see realistic use cases where we don't need to process an entire block trace but instead just want to decode some compact bytecode. - With the current public interface, the caller can only pass in an entire block trace to process. * Mpt trie panic refactor (#118) * refactor: refactoring mpt_trie to use more Results * fix: replace anyhow with this-error for mpt_trie * style: formatting * fix: fix results * fix: pr fixes * fix: fix error message * fix: format * fix: fix unusefull return type * fix: fix formatting * fix: pr fixes * fix: pr fixes * fix: pr fixes * tests: refactor some tests --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * refactor: remove some reallocations from decoder (#126) Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * Charge cold access cost in *CALL* before accessing state (#124) * Charge cold access cost in CALL before accessing state * PR feedback --------- Co-authored-by: BGluth <gluthb@gmail.com> * chore: add debug function for better logging in development (#134) * chore: add debug function for better logging in development * chore: fix clippy issue --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * Make test_receipt_encoding more meaningful. (#131) * Make test_receipt_encoding more meaningful. * Apply comment * Add a getter for the KERNEL codehash (#136) * Update CHANGELOG * Version bump for next release (#137) * Bumped sub-crate versions for a new release * Added in missing change log entries - Other PRs missed updating `CHANGELOG.md` * feat: swap out the internal U512 inside nibbles (#132) * feat: swap out the internal U512 inside nibbles * fix: comment fix * fix: fix clippy pr issues * fix: fix clippy issue * fix: fix pr comments * docs: update changelog * fix: update impl_to_nibbles --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * Some clippy fixes (#149) * MAX fixes for clippy * fix transmut without annotations * please the fmt gods * Remove interpreter-specific preinialization logic from State trait (#139) * Remove interpreter specific methods from State trait * Changelog * Make some more functions constant (#154) * Make some more functions constant * Update changelog * fix(keccak-sponge): properly constrain padding bytes (#158) * fix(keccak-sponge): properly constrain padding bytes * fix: block bytes offset * fix: constrain zero padding bytes * fix: use collect_vec * feat: replace is_final_input_len with is_padding_byte * fix: remove unnecessary iterators * Update evm_arithmetization/src/keccak_sponge/keccak_sponge_stark.rs Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Update evm_arithmetization/src/keccak_sponge/keccak_sponge_stark.rs Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Update evm_arithmetization/src/keccak_sponge/keccak_sponge_stark.rs Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Update evm_arithmetization/src/keccak_sponge/keccak_sponge_stark.rs Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Update evm_arithmetization/src/keccak_sponge/keccak_sponge_stark.rs Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Update evm_arithmetization/src/keccak_sponge/keccak_sponge_stark.rs Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * fix: remove redundant constraint * docs: define padding byte in comment --------- Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Reduce verbosity in logs (#160) * Reduce verbosity in logs * CHANGELOG * Add entry for 158 in CHANGELOG * Bump with latest starky (#161) * Bump with latest plonky2 * CHANGELOG * feat: decouple trace_decoder and proof_gen (#163) Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * Simplify withdrawals logic (#168) * Simplify withdrawals logic * Update CHANGELOG * Clippy * feat: extend trace decoder err info (#148) * feat: extend trace decoder err info * fix: fix clippy issue * feat: swap out the internal U512 inside nibbles (#132) * feat: swap out the internal U512 inside nibbles * fix: comment fix * fix: fix clippy pr issues * fix: fix clippy issue * fix: fix pr comments * docs: update changelog * fix: update impl_to_nibbles --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * Some clippy fixes (#149) * MAX fixes for clippy * fix transmut without annotations * please the fmt gods * fix: add pr comments fixes * fix: add pr comments fix * fix: add pr comment fix * docs: update changelog --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> Co-authored-by: Ben <bmarsh94@gmail.com> * Moved `Unreleased` changes to `0.3.0` (#173) - Also added two missing PRs --------- Co-authored-by: Hamy Ratoanina <hamy.ratoanina@toposware.com> Co-authored-by: wborgeaud <williamborgeaud@gmail.com> Co-authored-by: Vladimir Trifonov <vladimir-trifonov@users.noreply.github.com> Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> Co-authored-by: Linda Guiga <101227802+LindaGuiga@users.noreply.github.com> Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> Co-authored-by: Robin Salen <salenrobin@gmail.com> Co-authored-by: Ben <bmarsh94@gmail.com> Co-authored-by: Ayush Shukla <shuklaayush247@gmail.com>

* feat: extend trace decoder err info * fix: fix clippy issue * feat: swap out the internal U512 inside nibbles (#132) * feat: swap out the internal U512 inside nibbles * fix: comment fix * fix: fix clippy pr issues * fix: fix clippy issue * fix: fix pr comments * docs: update changelog * fix: update impl_to_nibbles --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * Some clippy fixes (#149) * MAX fixes for clippy * fix transmut without annotations * please the fmt gods * fix: add pr comments fixes * fix: add pr comments fix * fix: add pr comment fix * docs: update changelog --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> Co-authored-by: Ben <bmarsh94@gmail.com>

* Moved `Unreleased` changes to `0.3.0` (#173) - Also added two missing PRs * `main` --> `0.3.0` (#138) * Update plonky2 dependencies (#119) * Update plonky2 dependencies * Modify changelog * Charge gas before SLOAD and refactor `insert_accessed_storage_keys` (#117) * Charge gas before SLOAD and refactor `insert_accessed_storage_keys` * fmt * Only store value for cold access * PR feedback --------- Co-authored-by: BGluth <gluthb@gmail.com> * Increased the public interface for `trie_tools` (#123) - `trie_tools` needs to access a bit of currently private logic. Specifically, it needs to be able to process compact bytecode into `mpt_tries` directly. - I think this change is actually reasonable. I can see realistic use cases where we don't need to process an entire block trace but instead just want to decode some compact bytecode. - With the current public interface, the caller can only pass in an entire block trace to process. * Mpt trie panic refactor (#118) * refactor: refactoring mpt_trie to use more Results * fix: replace anyhow with this-error for mpt_trie * style: formatting * fix: fix results * fix: pr fixes * fix: fix error message * fix: format * fix: fix unusefull return type * fix: fix formatting * fix: pr fixes * fix: pr fixes * fix: pr fixes * tests: refactor some tests --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * refactor: remove some reallocations from decoder (#126) Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * Charge cold access cost in *CALL* before accessing state (#124) * Charge cold access cost in CALL before accessing state * PR feedback --------- Co-authored-by: BGluth <gluthb@gmail.com> * chore: add debug function for better logging in development (#134) * chore: add debug function for better logging in development * chore: fix clippy issue --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * Make test_receipt_encoding more meaningful. (#131) * Make test_receipt_encoding more meaningful. * Apply comment * Add a getter for the KERNEL codehash (#136) * Update CHANGELOG * Version bump for next release (#137) * Bumped sub-crate versions for a new release * Added in missing change log entries - Other PRs missed updating `CHANGELOG.md` * feat: swap out the internal U512 inside nibbles (#132) * feat: swap out the internal U512 inside nibbles * fix: comment fix * fix: fix clippy pr issues * fix: fix clippy issue * fix: fix pr comments * docs: update changelog * fix: update impl_to_nibbles --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * Some clippy fixes (#149) * MAX fixes for clippy * fix transmut without annotations * please the fmt gods * Remove interpreter-specific preinialization logic from State trait (#139) * Remove interpreter specific methods from State trait * Changelog * Make some more functions constant (#154) * Make some more functions constant * Update changelog * fix(keccak-sponge): properly constrain padding bytes (#158) * fix(keccak-sponge): properly constrain padding bytes * fix: block bytes offset * fix: constrain zero padding bytes * fix: use collect_vec * feat: replace is_final_input_len with is_padding_byte * fix: remove unnecessary iterators * Update evm_arithmetization/src/keccak_sponge/keccak_sponge_stark.rs Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Update evm_arithmetization/src/keccak_sponge/keccak_sponge_stark.rs Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Update evm_arithmetization/src/keccak_sponge/keccak_sponge_stark.rs Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Update evm_arithmetization/src/keccak_sponge/keccak_sponge_stark.rs Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Update evm_arithmetization/src/keccak_sponge/keccak_sponge_stark.rs Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Update evm_arithmetization/src/keccak_sponge/keccak_sponge_stark.rs Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * fix: remove redundant constraint * docs: define padding byte in comment --------- Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> * Reduce verbosity in logs (#160) * Reduce verbosity in logs * CHANGELOG * Add entry for 158 in CHANGELOG * Bump with latest starky (#161) * Bump with latest plonky2 * CHANGELOG * feat: decouple trace_decoder and proof_gen (#163) Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * Simplify withdrawals logic (#168) * Simplify withdrawals logic * Update CHANGELOG * Clippy * feat: extend trace decoder err info (#148) * feat: extend trace decoder err info * fix: fix clippy issue * feat: swap out the internal U512 inside nibbles (#132) * feat: swap out the internal U512 inside nibbles * fix: comment fix * fix: fix clippy pr issues * fix: fix clippy issue * fix: fix pr comments * docs: update changelog * fix: update impl_to_nibbles --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> * Some clippy fixes (#149) * MAX fixes for clippy * fix transmut without annotations * please the fmt gods * fix: add pr comments fixes * fix: add pr comments fix * fix: add pr comment fix * docs: update changelog --------- Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> Co-authored-by: Ben <bmarsh94@gmail.com> * Moved `Unreleased` changes to `0.3.0` (#173) - Also added two missing PRs --------- Co-authored-by: Hamy Ratoanina <hamy.ratoanina@toposware.com> Co-authored-by: wborgeaud <williamborgeaud@gmail.com> Co-authored-by: Vladimir Trifonov <vladimir-trifonov@users.noreply.github.com> Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> Co-authored-by: Linda Guiga <101227802+LindaGuiga@users.noreply.github.com> Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> Co-authored-by: Robin Salen <salenrobin@gmail.com> Co-authored-by: Ben <bmarsh94@gmail.com> Co-authored-by: Ayush Shukla <shuklaayush247@gmail.com> * Fix withdrawals accesses in state trie (#176) * Bumped `trace_decoder` version for new `zk_evm` release (#177) * Bumped `trace_decoder` version for new `zk_evm` release * Updated `CHANGELOG.md` * Revert "`main` --> `0.3.0` (#138)" (#179) This reverts commit 88d75ed. * Some cleanup (#190) * Cleanup perform_op * Move interpreter test methods to test module * Some more cleanup and removal * more cleanup * More cleanup misc and doc * Remove outdated arg * Fix clippy --------- Co-authored-by: BGluth <gluthb@gmail.com> Co-authored-by: Hamy Ratoanina <hamy.ratoanina@toposware.com> Co-authored-by: wborgeaud <williamborgeaud@gmail.com> Co-authored-by: Vladimir Trifonov <vladimir-trifonov@users.noreply.github.com> Co-authored-by: Vladimir Trifonov <trifonov.vp@gmail.com> Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> Co-authored-by: Robin Salen <salenrobin@gmail.com> Co-authored-by: Ben <bmarsh94@gmail.com> Co-authored-by: Ayush Shukla <shuklaayush247@gmail.com>

feat: extend trace decoder err info

bfb7733

vladimir-trifonov requested review from muursh and Nashtare as code owners April 5, 2024 13:26

github-actions bot added the crate: trace_decoder Anything related to the trace_decoder crate. label Apr 5, 2024

lastminutedev and others added 3 commits April 5, 2024 16:34

fix: fix clippy issue

bb46206

Some clippy fixes (#149)

f06be46

* MAX fixes for clippy * fix transmut without annotations * please the fmt gods

vladimir-trifonov requested a review from wborgeaud as a code owner April 8, 2024 12:29

Merge branch 'develop' into extend_trace_decoder_err_info

8a07a0e

BGluth approved these changes Apr 9, 2024

View reviewed changes

trace_decoder/src/decoding.rs Outdated Show resolved Hide resolved

trace_decoder/src/decoding.rs Show resolved Hide resolved

trace_decoder/src/decoding.rs Show resolved Hide resolved

trace_decoder/src/decoding.rs Show resolved Hide resolved

Nashtare linked an issue Apr 9, 2024 that may be closed by this pull request

Add additional metadata to decoder errors #121

Closed

lastminutedev and others added 2 commits April 9, 2024 10:39

fix: add pr comments fixes

ee0fabb

Merge branch 'develop' into extend_trace_decoder_err_info

7c06cef

BGluth approved these changes Apr 9, 2024

View reviewed changes

muursh reviewed Apr 11, 2024

View reviewed changes

trace_decoder/src/decoding.rs Show resolved Hide resolved

lastminutedev and others added 3 commits April 15, 2024 13:41

fix: add pr comments fix

cada63a

fix: add pr comment fix

24b6da3

Merge branch 'develop' into extend_trace_decoder_err_info

63fe8f0

vladimir-trifonov requested a review from muursh April 16, 2024 08:58

Merge branch 'develop' into extend_trace_decoder_err_info

1bb1985

BGluth reviewed Apr 16, 2024

View reviewed changes

BGluth approved these changes Apr 16, 2024

View reviewed changes

Merge branch 'develop' into extend_trace_decoder_err_info

52c1034

muursh approved these changes Apr 18, 2024

View reviewed changes

Nashtare approved these changes Apr 18, 2024

View reviewed changes

docs: update changelog

54a9480

Merge branch 'develop' into extend_trace_decoder_err_info

8c05701

muursh merged commit 4a9dfda into develop Apr 18, 2024
6 checks passed

muursh deleted the extend_trace_decoder_err_info branch April 18, 2024 16:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: extend trace decoder err info #148

feat: extend trace decoder err info #148

vladimir-trifonov commented Apr 5, 2024

BGluth left a comment

BGluth Apr 16, 2024

Nashtare left a comment

muursh commented Apr 18, 2024

vladimir-trifonov commented Apr 18, 2024

feat: extend trace decoder err info #148

feat: extend trace decoder err info #148

Conversation

vladimir-trifonov commented Apr 5, 2024

BGluth left a comment

Choose a reason for hiding this comment

BGluth Apr 16, 2024

Choose a reason for hiding this comment

Nashtare left a comment

Choose a reason for hiding this comment

muursh commented Apr 18, 2024

vladimir-trifonov commented Apr 18, 2024