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

client/rpc: Add new RPC for current transaction pool status #7334

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vatsa287
Copy link
Contributor

@vatsa287 vatsa287 commented Jan 24, 2025

Description

This PR adds a new RPC author_poolStatus which shows the current status of the substrate transaction pool.

During one of the implementation of a queue which submits batch transaction in thousands, I did not find a way of getting the status of the pool. We set the pool limit in transactions and its size with --pool-limit & --pool-kbytes respectively.
I wanted a way to find if the transaction pool is nearing its limit or not before any new bulk transactions are sent to the pool so the transaction do not get bounced, which is my current case.

I have used author_pendingExtrinsics RPC but it does not give the bytes occupied in the pool. Since bytes occupied is also another crucial factor along with count for the limit to be enforced whether the pool is full or not.
Another downside of using this RPC is that that response is too big adding unnecessary load to the nodes and to parse the data as well.

Since we are already using status() method at various places of TransactionPool trait. Send the data as a RPC too, benefitting with low response size.

In future I wish to add the current limits on count and size to this response as well. For now I will assume that transaction-pool will be full when ready.count + future.count(10% of ready.count) > pool_limit or ready.total_bytes + future.total_bytes(10% of ready.total_bytes) > pool_limit_bytes.

Review Notes

Utilise the status() method of TransactionPool trait to expose that data as part of a new RPC call named poolStatus as part of author.
I have added tests to demonstrate the same.

Signed-off-by: Shreevatsa N [email protected]

@paritytech-review-bot paritytech-review-bot bot requested a review from a team January 24, 2025 16:53
@vatsa287
Copy link
Contributor Author

vatsa287 commented Jan 24, 2025

Tests from sc-rpc for reference.
Name of the test method: `test author::tests::author_should_return_pool_status ... ok

vatsa@Shreevatsas-MacBook-Pro polkadot-sdk % cargo test -p sc-rpc                             
   Compiling sc-rpc v29.0.0 (/Users/vatsa/polkadot-sdk/substrate/client/rpc)
   Compiling sc-rpc-spec-v2 v0.34.0 (/Users/vatsa/polkadot-sdk/substrate/client/rpc-spec-v2)
   Compiling sc-service v0.35.0 (/Users/vatsa/polkadot-sdk/substrate/client/service)
   Compiling substrate-test-client v2.0.1 (/Users/vatsa/polkadot-sdk/substrate/test-utils/client)
   Compiling substrate-test-runtime v2.0.0 (/Users/vatsa/polkadot-sdk/substrate/test-utils/runtime)
   Compiling substrate-test-runtime-client v2.0.0 (/Users/vatsa/polkadot-sdk/substrate/test-utils/runtime/client)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 18.55s
     Running unittests src/lib.rs (target/debug/deps/sc_rpc-0b5432d9b553d77b)

running 55 tests
test author::tests::author_should_return_watch_validation_error ... ok
test author::tests::author_has_key ... ok
test author::tests::author_should_rotate_keys ... ok
test author::tests::author_should_return_pool_status ... ok
test author::tests::author_should_remove_extrinsics ... ok
test author::tests::author_should_insert_key ... ok
test author::tests::author_should_return_pending_extrinsics ... ok
test author::tests::author_should_watch_extrinsic ... ok
test author::tests::author_has_session_keys ... ok
test chain::tests::should_return_a_block ... ok
test chain::tests::should_return_block_hash ... ok
test author::tests::author_submit_transaction_should_not_cause_error ... ok
test offchain::tests::local_storage_should_work ... ok
test offchain::tests::offchain_calls_considered_unsafe ... ok
test chain::tests::should_notify_about_finalized_block ... ok
test chain::tests::should_notify_about_latest_block ... ok
test state::tests::should_deserialize_storage_key ... ok
test chain::tests::should_notify_about_best_block ... ok
test state::tests::concrete_storage_subscriptions_are_rpc_safe ... ok
test state::tests::should_call_contract ... ok
test chain::tests::should_return_header ... ok
test state::tests::should_notify_on_runtime_version_initially ... ok
test dev::tests::deny_unsafe_works ... ok
test dev::tests::block_stats_work ... ok
test state::tests::should_notify_about_storage_changes ... ok
test chain::tests::should_return_finalized_hash ... ok
test state::tests::should_return_child_storage_entries ... ok
test state::tests::should_return_runtime_version ... ok
test system::tests::system_chain_works ... ok
test system::tests::system_health ... ok
test system::tests::system_local_listen_addresses_works ... ok
test system::tests::system_local_peer_id_works ... ok
test system::tests::system_name_works ... ok
test system::tests::system_network_add_reserved ... ok
test system::tests::system_network_remove_reserved ... ok
test system::tests::system_network_reserved_peers ... ok
test system::tests::system_network_state ... ok
test system::tests::system_node_roles ... ok
test system::tests::system_peers ... ok
test system::tests::system_properties_works ... ok
test system::tests::system_sync_state ... ok
test system::tests::system_type_works ... ok
test system::tests::system_version_works ... ok
test state::tests::wildcard_storage_subscriptions_are_rpc_unsafe ... ok
test utils::tests::pipe_from_stream_with_bounded_vec ... ok
test utils::tests::pipe_from_stream_works ... ok
test state::tests::should_return_storage_entries ... ok
test utils::tests::subscription_is_dropped_when_stream_is_empty ... ok

running 1 test
test state::tests::should_return_storage ... ok
test state::tests::should_return_child_storage ... ok
test state::tests::should_send_initial_storage_changes_and_notifications ... ok
test state::utils::tests::spawn_blocking_with_timeout_works ... ok
test system::tests::test_add_reset_log_filter ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 54 filtered out; finished in 0.22s

test system::tests::test_add_reset_log_filter ... ok
test state::tests::should_query_storage ... ok
test utils::tests::subscription_replace_old_messages ... ok

test result: ok. 55 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 20.54s

   Doc-tests sc_rpc

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Live RPC testing:

curl -H "Content-Type: application/json" -d '{
  "jsonrpc": "2.0",
  "id": 1,
  "method": "author_poolStatus",
  "params": []
}' http://127.0.0.1:65111

{"jsonrpc":"2.0","id":1,"result":{"ready":0,"ready_bytes":0,"future":0,"future_bytes":0}}% 

@vatsa287
Copy link
Contributor Author

vatsa287 commented Jan 24, 2025

I don't know exactly which label to be requested to be added, I am guessing T3-RPC-API!?

Signed-off-by: Shreevatsa N <[email protected]>
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

See: #7138

I'm not really happy to do this, but also discussed this a little bit more with @michalkucharczyk in DMs. Generally, if this would be added via the new RPC-api spec, I would lift my "No".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants