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

fix: rpc result serde and avoid rate limits #4325

Merged
merged 2 commits into from Aug 24, 2023

Conversation

agostbiro
Copy link
Collaborator

This PR started out as a quickfix for Block<Transaction> deserialization that was blocking @Wodann , but then some new issues popped up for which I also included fixes. Specifically, this PR contains:

  • Make it possible for Block<Transaction> to be deserialized from serde_json::Value
  • Make it possible for TypedReceipt<Log> to be deserialized from serde_json::Value
  • Fix deserialization for yParity in Block. Geth recently started to return yParity alongside the v parameter for EIP-2930 and EIP-1559 transactions even though these should only contain the yParity parameter in theory. I discovered this when fetching blocks from Infura.
  • Join some remote tests in the rpc client to reduce requests to avoid getting rated limited
  • Limit concurrent requests for transaction receipts to avoid getting rate limited
  • Use shared cache for integration tests and make them sequential to avoid getting rate limited

@changeset-bot
Copy link

changeset-bot bot commented Aug 24, 2023

⚠️ No Changeset found

Latest commit: 0688258

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Aug 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hardhat ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 24, 2023 9:21pm
hardhat-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 24, 2023 9:21pm

@@ -1070,13 +1070,27 @@ mod tests {
}

#[tokio::test]
async fn get_earliest_block() {
async fn get_block_with_transaction_data() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be renamed to signal that you're testing caching?

pub v: u64,
/// Y-parity for EIP-2930 and EIP-1559 transactions. In theory these transactions types
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did this cause a crash when deserializing?

@Wodann Wodann added type:bug Something isn't working area:edr labels Aug 24, 2023
@Wodann Wodann merged commit cd2213f into rethnet/main Aug 24, 2023
19 of 22 checks passed
@Wodann Wodann deleted the rethnet/transaction-deser branch August 24, 2023 22:07
@Wodann
Copy link
Collaborator

Wodann commented Aug 25, 2023

I merged as I only had one tiny nitpick and it seemed prudent to get this merged. It's still worth having a look at this question, @agostbiro: #4325 (comment)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:edr type:bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants