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

Maximum tx size enforcement does not account for full transaction encoding #2560

Closed
5 tasks
mzabaluev opened this issue Aug 15, 2022 · 0 comments · Fixed by #2575
Closed
5 tasks

Maximum tx size enforcement does not account for full transaction encoding #2560

mzabaluev opened this issue Aug 15, 2022 · 0 comments · Fixed by #2575
Milestone

Comments

@mzabaluev
Copy link
Contributor

Summary of Bug

When splitting messages into batches to form transactions, the max_tx_size limit only accounts for message content. It's still possible that the full TxRaw encoded message exceeds this limit.

This raises some questions:

  • Does this configuration parameter mean the full transaction size including the signatures and AuthInfo data, as encoded in protobuf? Or is there any other way to calculate the tx size that could be more useful to relayer operators, e.g. for purposes of limiting fees?
  • How likely does this discrepancy matter in practical use, to decide on the priority of fixing this?
  • How important it is to pack the messages exhaustively up to the tx size limit? For example, if the relayer bumps one message off a transaction to the next batch due to using some size estimation heuristics, while in perfect implementation the message could still fit within the tx, how costly this miscalculation could be?

Version

1.0.0-rc.2

Steps to Reproduce

Not observed specifically for this cause, but see #2422 for possible symptoms.

Acceptance Criteria

  • There is a documented meaning for the max_tx_size chain config parameter, and it is correctly enforced.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
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 a pull request may close this issue.

2 participants