Skip to content
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

introduce new read-only transaction RPC end point #558

Merged
merged 43 commits into from
Mar 2, 2023
Merged

Conversation

linh2931
Copy link
Member

@linh2931 linh2931 commented Dec 8, 2022

Resolved #440

This PR introduces a new read-only transaction RPC end point. Major features are

  • Provide a new endpoint send_read_only_transaction
  • Provide a cleos push transaction option --read for pushing read-only transaction
  • Provide a cleos push action option --read for pushing read-only action
  • Do not sign any read-only transactions by cleos
  • Do not allow any permissions for read-only transactions
  • Assert any attempt to modify DB tables using DB API host functions
  • Assert any attempt to modify DB tables using native functions
  • Do not modify global_action_sequence and recv_sequence
  • Do not do deepmind logging
  • Add a catch-all flag in Chainbase to prevent any unlikely missed DB modification attempts.
  • Add integration tests for cleos/chain_plugin/producer_plugin.
  • Add unit tests for internal logic

Please note, the Chainbase submodule commit will be updated after corresponding Chainbase PR AntelopeIO/chainbase#4 is merged.

libraries/chain/apply_context.cpp Outdated Show resolved Hide resolved
libraries/chain/eosio_contract.cpp Show resolved Hide resolved
libraries/chain/apply_context.cpp Outdated Show resolved Hide resolved
libraries/chain/transaction_context.cpp Outdated Show resolved Hide resolved
libraries/chain/controller.cpp Outdated Show resolved Hide resolved
libraries/chain/transaction_context.cpp Show resolved Hide resolved
plugins/chain_plugin/chain_plugin.cpp Show resolved Hide resolved
plugins/chain_plugin/chain_plugin.cpp Outdated Show resolved Hide resolved
plugins/producer_plugin/producer_plugin.cpp Show resolved Hide resolved
programs/cleos/main.cpp Show resolved Hide resolved
@@ -289,7 +289,7 @@ void apply_context::require_recipient( account_name recipient ) {
schedule_action( action_ordinal, recipient, false )
);

if (auto dm_logger = control.get_deep_mind_logger()) {
if (auto dm_logger = control.get_deep_mind_logger(); dm_logger && !trx_context.is_read_only()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of repeating this pattern many times, why not create a new member function on transaction_context like:

deep_mind_handler* get_dm_logger() {
    return is_read_only() ? nullptr : control.get_deep_mind_logger();
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your suggestion. Since read-only is an exception to deep-mind logging, it helps to understand the code by specifically showing when we don't do deep-mind logging.

…lify code, do not allow non-zero delay_sec; do not do validate_expiration and validate_tapo; add back elapsed and net_usage to trace
libraries/chain/transaction_context.cpp Outdated Show resolved Hide resolved
…ransient directly to indicate doing deep mind logging or not
libraries/chain/authorization_manager.cpp Outdated Show resolved Hide resolved
libraries/chain/authorization_manager.cpp Outdated Show resolved Hide resolved
libraries/chain/controller.cpp Outdated Show resolved Hide resolved
libraries/chain/controller.cpp Outdated Show resolved Hide resolved
libraries/chain/controller.cpp Show resolved Hide resolved
libraries/chain/controller.cpp Show resolved Hide resolved
libraries/chain/controller.cpp Show resolved Hide resolved
libraries/chain/controller.cpp Show resolved Hide resolved
libraries/chain/apply_context.cpp Outdated Show resolved Hide resolved
libraries/chain/apply_context.cpp Outdated Show resolved Hide resolved
libraries/chain/resource_limits.cpp Outdated Show resolved Hide resolved
libraries/chain/webassembly/privileged.cpp Show resolved Hide resolved
Copy link
Member

@arhag arhag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approved. But please fix the three remaining issues (two error strings, and the comments repeated at four different lines) before merging.

libraries/chain/apply_context.cpp Outdated Show resolved Hide resolved
libraries/chain/apply_context.cpp Outdated Show resolved Hide resolved
libraries/chain/controller.cpp Outdated Show resolved Hide resolved
@linh2931 linh2931 merged commit 3c0e506 into main Mar 2, 2023
@linh2931 linh2931 deleted the send_read_only_trx branch March 2, 2023 13:22
@linh2931 linh2931 added the documentation Improvements or additions to documentation label Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New RPC endpoint send_read_only_transaction
4 participants