Process configchange event and save the parameters in some data structure. #190
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
This reverts commit 70ef2e3.
elmato
reviewed
Mar 1, 2024
src/channels.hpp
Outdated
| std::vector<native_trx> transactions; | ||
| }; | ||
|
|
||
| struct consensus_parameter_event { |
There was a problem hiding this comment.
Is this structure used anywhere?
There was a problem hiding this comment.
forgot to delete. deleted
src/block_conversion_plugin.hpp
Outdated
| @@ -19,6 +19,17 @@ struct evmtx_v0 { | |||
| using evmtx_type = std::variant<evmtx_v0>; | |||
| EOSIO_REFLECT(evmtx_v0, eos_evm_version, rlpx) | |||
|
|
|||
| struct gas_parameter_data_v1 { | |||
There was a problem hiding this comment.
We should replicate the types we have here until we have a place where to put all these interfaces
There was a problem hiding this comment.
Yeah, was copied from there back then. Didn't notice it was changed.
elmato
reviewed
Mar 5, 2024
src/block_conversion_plugin.cpp
Outdated
| .gas_sset = std::visit([](auto&& arg) -> auto& { return arg.gas_parameter.gas_sset; }, new_config), | ||
| } | ||
| }); | ||
| curr.consensus_parameter_index = curr.header.number; |
There was a problem hiding this comment.
We should save here the consensus parameters to db
src/blockchain_plugin.cpp
Outdated
Comment on lines
50
to
78
| if (new_block->consensus_parameter_index) { | ||
| if (*new_block->consensus_parameter_index == new_block->header.number) { | ||
| // Parameters updated in this block. | ||
| SILKWORM_ASSERT(new_block->consensus_parameters_cache.has_value()); | ||
| update_consensus_parameters(exec_engine->get_tx(), new_block->header.number, *new_block->consensus_parameters_cache); | ||
| last_consensus_parameters = new_block->consensus_parameters_cache; | ||
| last_consensus_parameters_index = new_block->consensus_parameter_index; | ||
| } | ||
| else if (last_consensus_parameters_index && *last_consensus_parameters_index == *new_block->consensus_parameter_index) { | ||
| // Copy over parameters for last block. | ||
| SILKWORM_ASSERT(last_consensus_parameters.has_value()); | ||
| new_block->consensus_parameters_cache = last_consensus_parameters; | ||
| } else { | ||
| // Parameter not cached, happen during startup or fork. | ||
| new_block->consensus_parameters_cache = read_consensus_parameters(exec_engine->get_tx(), *new_block->consensus_parameter_index); | ||
| // In such cases, the parameter MUST be in db. | ||
| SILKWORM_ASSERT(new_block->consensus_parameters_cache.has_value()); | ||
|
|
||
| last_consensus_parameters_index = new_block->consensus_parameter_index; | ||
| last_consensus_parameters = new_block->consensus_parameters_cache; | ||
| } | ||
| } | ||
| else { | ||
| // Should only reach here if fork happens during version update. | ||
| // TODO: Add guards according to eosevm versions. | ||
| last_consensus_parameters_index = std::nullopt; | ||
| last_consensus_parameters = std::nullopt; | ||
| } | ||
|
|
There was a problem hiding this comment.
We don't need this and the last_consensus_parameters_index / last_consensus_parameters variables
src/blockchain_plugin.cpp
Outdated
Comment on lines
120
to
121
| std::optional<silkworm::BlockNum> last_consensus_parameters_index; | ||
| std::optional<eosevm::ConsensusParameters> last_consensus_parameters; |
elmato
approved these changes
Mar 16, 2024
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.
Resolve #186
The planned implementation is like this:
1 ship_receiver_plugin will check if it received a configchange event, if found, make sure it's the first related action in a block, save the new config in the resulting native_block/
2 block_conversion_plugin will make sure if there's a new config in the incoming native_block, it should be the first one native block for a resulting evm block. Then:
a save the new config in the resulting evm block
b change the evm block's consensus_parameter_index to it's own height.
3 blockchain_plugin will detect if there's new config coming in with the evm blocks. if so:
a save new parameters
b verify_chain for the blocks before if necessary
c insert the incoming blcok
d verify chain (may be not necessary)
-- EDIT: after some double thinking, we do not need this for blockchain_plugin, let's just try a normal process first.