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: enforce fallback response integrity & evm user error matching #861

Merged
merged 2 commits into from
May 31, 2022

Conversation

crisog
Copy link
Contributor

@crisog crisog commented May 30, 2022

The fallback responses are the ones coming from our backup system. They come into play when the node selected from the network can't do it's job, and doesn't return any response. In our mission to be as fast as we can, we use our internal system to serve that request (doesn't generate revenue).

These responses were only enforced to a certain format, but the truth is that there are other several server errors from blockchain's node clients that indicate that the call wasn't effective. This PR adds some additional text enforcement to abstract these errors from users and let it fail without any specific blockchain's node message that could add confusion to end user.

Example error:

{"error":{"code":-32000,"message":"rpc error: code = Unavailable desc = connection error: desc = \"transport: Error while dialing dial tcp 127.0.0.1:9090: connect: connection refused\""},"id":732830328053361800,"jsonrpc":"2.0"}

@height
Copy link

height bot commented May 30, 2022

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@crisog
Copy link
Contributor Author

crisog commented May 31, 2022

Another fix was also introduced at this PR, since it's error-related. It's the fix to the problem outlined in #862.

The current approach is matching server errors, and user error is anything with -32k error and NOT a server error. Since server errors are fewer, it is easier to handle and node runners won't be penalized by false positives on EVM unknown user error messages.

Comment on lines +467 to +469
if (isRelayError(stringifiedResponse) && !isUserError(stringifiedResponse)) {
throw new Error(`Response is not valid: ${stringifiedResponse}`)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sure that the response obtained from altruist is a good one


// 3 is execution error
// -32000 itself can be thrown for what are server errors
// all other -32000 are user errors
return userError || code === 3 || code < -32000
return (!serverError && code === -32000) || code === 3 || code < -32000
Copy link
Contributor Author

@crisog crisog May 31, 2022

Choose a reason for hiding this comment

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

anything thats not a server error & has error code -32k is assumed to be an user error, not node error.

@crisog crisog changed the title fix: enforce fallback responses fix: enforce fallback response integrity & evm user error matching May 31, 2022
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