Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Set transaction size limit #4107
Set transaction size limit #4107
Changes from 18 commits
c5e4290
c1312d6
caf9c77
5bb8eea
29a1ee7
83e8cc9
2a627ee
552ddd1
e8e8e7f
98b49f4
4956d18
b87a56a
9ea73c7
f379ff7
030c85a
69ead0e
d28002f
1cc7e7e
b649ab5
7eee611
8dfbc0f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check should only be done once. There is no reason to check transactions for which you already checked. For example, inside
prepare_transactions
it doesn't make sense to check again because this check should have been done when the transaction is received. To further reduce the overhead, you could changeNetworkClientMessage::Transaction
to include the transaction size. It probably even makes sense to implement custom borsh deserializer that would fail when the a larger-than-allowed transaction arrives on the network (this can potentially be done in a separate PR)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that at least one serialization is already happening inside
SignedTransaction::new
, to saveTransaction
hash. So I savedsize
there in the same fashion and started to use it, which shouldn't give significant overhead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that makes sense