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.

Sign up for GitHub

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

[pallet-revive] revive json-rpc small adjustments #6309

Closed
wants to merge 3 commits into from
Closed

[pallet-revive] revive json-rpc small adjustments #6309

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f1e5537
[pallet-revive] revive json-rpc small adjustments
pgherveou Oct 31, 2024
20fa632
Update from pgherveou running command 'prdoc --audience runtime_dev -…
actions-user Oct 31, 2024
f5652bd
Update prdoc/pr_6309.prdoc
pgherveou Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions prdoc/pr_6309.prdoc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
title: '[pallet-revive] revive json-rpc small adjustments'
doc:
- audience: Runtime Dev
description: |-
Small adjustments to eth-rpc
- bump cache size
- use essential-tasks to spawn the json-rpc block subsrciption tasks
pgherveou marked this conversation as resolved.
Show resolved Hide resolved
crates:
- name: sc-service
bump: patch
- name: pallet-revive-eth-rpc
bump: patch
4 changes: 3 additions & 1 deletion substrate/client/service/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ pub use sc_transaction_pool::TransactionPoolOptions;
pub use sc_transaction_pool_api::{error::IntoPoolError, InPoolTransaction, TransactionPool};
#[doc(hidden)]
pub use std::{ops::Deref, result::Result, sync::Arc};
pub use task_manager::{SpawnTaskHandle, Task, TaskManager, TaskRegistry, DEFAULT_GROUP_NAME};
pub use task_manager::{
SpawnEssentialTaskHandle, SpawnTaskHandle, Task, TaskManager, TaskRegistry, DEFAULT_GROUP_NAME,
};
use tokio::runtime::Handle;

const DEFAULT_PROTOCOL_ID: &str = "sup";
Expand Down
6 changes: 3 additions & 3 deletions substrate/frame/revive/rpc/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,11 @@ pub fn run(cmd: CliCommand) -> anyhow::Result<()> {
let tokio_handle = tokio_runtime.handle();
let signals = tokio_runtime.block_on(async { Signals::capture() })?;
let mut task_manager = TaskManager::new(tokio_handle.clone(), prometheus_registry)?;
let spawn_handle = task_manager.spawn_handle();
let essential_spawn_handle = task_manager.spawn_essential_handle();

let gen_rpc_module = || {
let signals = tokio_runtime.block_on(async { Signals::capture() })?;
let fut = Client::from_url(&node_rpc_url, &spawn_handle).fuse();
let fut = Client::from_url(&node_rpc_url, &essential_spawn_handle).fuse();
pin_mut!(fut);

match tokio_handle.block_on(signals.try_until_signal(fut)) {
Expand All @@ -125,7 +125,7 @@ pub fn run(cmd: CliCommand) -> anyhow::Result<()> {

// Prometheus metrics.
if let Some(PrometheusConfig { port, registry }) = prometheus_config.clone() {
spawn_handle.spawn(
task_manager.spawn_handle().spawn(
"prometheus-endpoint",
None,
prometheus_endpoint::init_prometheus(port, registry).map(drop),
Expand Down
8 changes: 5 additions & 3 deletions substrate/frame/revive/rpc/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ use pallet_revive::{
},
EthContractResult,
};
use sc_service::SpawnTaskHandle;
use sp_runtime::traits::{BlakeTwo256, Hash};
use sp_weights::Weight;
use std::{
Expand Down Expand Up @@ -165,7 +164,7 @@ impl From<ClientError> for ErrorObjectOwned {

/// The number of recent blocks maintained by the cache.
/// For each block in the cache, we also store the EVM transaction receipts.
pub const CACHE_SIZE: usize = 10;
pub const CACHE_SIZE: usize = 256;
Copy link
Member

Choose a reason for hiding this comment

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

How did you come up with this number? Might be worth to explain it in above comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just a random number :)
that's how many block we keep in the cache for answering eth json-rpc requests, 10 was probably too low
so bumping it just to be safe, I will probably revisit that later

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the memory used in the worst-case scenario for caching one block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

did not check but it should be pretty light, we can adapt this code later on so that it does not grow more than let's say 100mb


impl<const N: usize> BlockCache<N> {
fn latest_block(&self) -> Option<&Arc<SubstrateBlock>> {
Expand Down Expand Up @@ -344,7 +343,10 @@ async fn extract_block_timestamp(block: &SubstrateBlock) -> Option<u64> {
impl Client {
/// Create a new client instance.
/// The client will subscribe to new blocks and maintain a cache of [`CACHE_SIZE`] blocks.
pub async fn from_url(url: &str, spawn_handle: &SpawnTaskHandle) -> Result<Self, ClientError> {
pub async fn from_url(
url: &str,
spawn_handle: &sc_service::SpawnEssentialTaskHandle,
) -> Result<Self, ClientError> {
log::info!(target: LOG_TARGET, "Connecting to node at: {url} ...");
let inner: Arc<ClientInner> = Arc::new(ClientInner::from_url(url).await?);
log::info!(target: LOG_TARGET, "Connected to node at: {url}");
Expand Down