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

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

Closed
wants to merge 3 commits into from

Conversation

pgherveou
Copy link
Contributor

@pgherveou pgherveou commented Oct 31, 2024

Small adjustments to eth-rpc

  • bump cache size
  • use essential-tasks to spawn the json-rpc block subsrciption tasks

@pgherveou pgherveou added R0-silent Changes should not be mentioned in any release notes T7-smart_contracts This PR/Issue is related to smart contracts. labels Oct 31, 2024
@pgherveou
Copy link
Contributor Author

/cmd prdoc --audience runtime_dev --bump patch

@pgherveou pgherveou enabled auto-merge October 31, 2024 09:04
@@ -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

prdoc/pr_6309.prdoc Outdated Show resolved Hide resolved
Co-authored-by: Niklas Adolfsson <[email protected]>
@pgherveou
Copy link
Contributor Author

pgherveou commented Nov 2, 2024

cherry picking these changes into a single PR here #6317

@pgherveou pgherveou closed this Nov 2, 2024
auto-merge was automatically disabled November 2, 2024 08:22

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T7-smart_contracts This PR/Issue is related to smart contracts.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants