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

GetAccountRange response limitHash and responseBytes spec adherence fix #30508

Conversation

infosecual
Copy link
Contributor

This PR fixes two minor issues in ServiceGetAccountRangeQuery which services GetAccountRange requests when peers are requesting accounts for during snap sync.

The first issue is that the logic to prevent serving a hash larger than limitHash has an off-by-one:

// If we've exceeded the request threshold, abort
if bytes.Compare(hash[:], req.Limit[:]) >= 0 {
	break
}

The Devp2p Spec defines limitHash as the "Account hash after which to stop serving data". Changing the conditional to > instead of >= allows us to serve the limitHash account according to the spec.

The second issue this fixes is that geth currently serves above the responseBytes limit by the size of one account. This is because the size check currently happens after the account is appended to the response. The suggested fix is to move these conditional checks to before the account is appended to the response.

@infosecual
Copy link
Contributor Author

Making a new PR for this

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.

1 participant