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

Validate rpc response consistency #1150

Closed
kyscott18 opened this issue Oct 9, 2024 · 5 comments · Fixed by #1152
Closed

Validate rpc response consistency #1150

kyscott18 opened this issue Oct 9, 2024 · 5 comments · Fixed by #1152
Labels
A: Sync engine Area: Sync engine T: Feature Type: Feature

Comments

@kyscott18
Copy link
Collaborator

Description

Before further processing, we should validate that all referenced data is present. All blockHashs and transactionHashs should have a corresponding block and transaction. This should happen in both sync-historical and sync-realtime.

Related

Several users have reported that transaction is undefined in their ponder indexing functions. After further investigation this is a result of the database being corrupted, with a log.transactionHash referencing a transaction that doesn't exist

@kyscott18 kyscott18 added T: Feature Type: Feature A: Sync engine Area: Sync engine labels Oct 9, 2024
@khaidarkairbek
Copy link
Contributor

What should be the proper handling if the log references the transaction that is not in the block? Should it throw an error?

@kyscott18
Copy link
Collaborator Author

What should be the proper handling if the log references the transaction that is not in the block? Should it throw an error?

Great question - I think the sync-historical logic should log an error containing exactly what is inconsistent and then fail. sync-realtime should log a similar warning, and then enter this retry loop that is already used to handle some rpc-related errors.

} catch (_error) {
if (isKilled) return;
const error = _error as Error;
error.stack = undefined;
args.common.logger.warn({
service: "realtime",
msg: `Failed to process '${args.network.name}' block ${hexToNumber(block.number)}`,
error,
});

The reason for the different behavior in sync-historical vs sync-realtime is that json rpc providers are generally less reliable (because of load balancing or reorg handilng logic or whatever reason) when the requested data is closer to the tip of the chain but eventually able to serve the correct data after retrying. Farther back in the historical range the providers are more reliable but errors are persistent.

@khaidarkairbek
Copy link
Contributor

I see, can i take over this issue btw?

@kyscott18
Copy link
Collaborator Author

kyscott18 commented Oct 9, 2024

Yes, that would be great. I think you should start with the sync-realtime, cause it should be more straightforward than the sync-historical, and make a draft pr when you finish that part.

@kyscott18
Copy link
Collaborator Author

@khaidarkairbek Here's another spot in the same function we would want to update that does some rpc response validation and throws and error:

if (block.logsBloom !== zeroLogsBloom && logs.length === 0) {
throw new Error(
`Detected invalid '${args.network.name}' eth_getLogs response.`,
);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: Sync engine Area: Sync engine T: Feature Type: Feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants