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

Make tx size limits consistent across the codebase #3765

Closed
marta-lokhova opened this issue Jun 6, 2023 · 2 comments · Fixed by #3786 or #3937
Closed

Make tx size limits consistent across the codebase #3765

marta-lokhova opened this issue Jun 6, 2023 · 2 comments · Fixed by #3786 or #3937
Assignees
Labels

Comments

@marta-lokhova
Copy link
Contributor

This issue tracks a slight inconsistency between the tx queue and overlay when it comes to validating transactions against max allowed size. Specifically, tx queue takes into account envelope size only (

auto txSize = xdr::xdr_size(mEnvelope.v1().tx);
), while overlay computes the size of the whole message (which includes signatures)
return static_cast<uint64_t>(xdr::xdr_argpack_size(msg));

We should ensure we validate size consistently to prevent any unnecessary drops at the overlay layer.

@dmkozh
Copy link
Contributor

dmkozh commented Jun 14, 2023

We'll use full tx size for the fees, so there won't be discrepancy. I'll keep this open to update when the change is merged.

@dmkozh dmkozh self-assigned this Jun 14, 2023
@dmkozh dmkozh mentioned this issue Jun 20, 2023
6 tasks
@marta-lokhova marta-lokhova reopened this Sep 14, 2023
@marta-lokhova
Copy link
Contributor Author

@dmkozh Looks like there are still discrepancies in two places:

I thought the idea was to use full tx size across codebase; the limit itself is bandwidth-related, and we never flood just the tx portion of the tx envelope, so it makes sense to count the whole transaction. If we don't, this is also problematic for overlay as it relies on txMaxSizeBytes for calculations that expect full tx size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants