SHiP: Auto disable-replay-opts instead of exiting with error #1467
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… not actually needed for trace data.
greg7mdp
approved these changes
Jul 28, 2023
libraries/state_history/include/eosio/state_history/serialization.hpp
Outdated
Show resolved
Hide resolved
libraries/state_history/include/eosio/state_history/serialization.hpp
Outdated
Show resolved
Hide resolved
| @@ -35,11 +49,27 @@ struct history_context_wrapper { | |||
| }; | |||
|
|
|||
| template <typename P, typename T> | |||
| history_context_wrapper<std::decay_t<P>, std::decay_t<T>> make_history_context_wrapper(const chainbase::database& db, | |||
| const P& context, const T& obj) { | |||
| struct history_context_wrapper_stateless { | |||
There was a problem hiding this comment.
I think I would have made history_context_wrapper_stateless a base of history_context_wrapper.
the advantage is that the functions accepting a history_context_wrapper_stateless& would also take a history_context_wrapper&, although I'm not sure if it would have helped a lot.
linh2931
reviewed
Jul 31, 2023
We have no unittests and very little integration tests that would test any of the serialization logic. Before 3.2 we had no tests for SHiP. We have separate issues for adding tests for SHiP. I think creating tests for serialization is out of scope for this PR. |
greg7mdp
approved these changes
Jul 31, 2023
linh2931
approved these changes
Jul 31, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
state_history_pluginto automatically setdisable-replay-opts=trueinstead of indicating an error and exiting.disable-replay-opts=trueonly whenchain-state-history=trueis enabled as it is not needed ifstate_history_pluginis running with onlytrace-history=trueenabled.chainbasewhen serializing transaction trace data. This verifies thatdisable-replay-opts=trueis not needed if only logging transaction trace data.disable-replay-optsto be set inconfig.iniinstead of only on the command line.Resolves #1403