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
rpc/validation: enable packages through testmempoolaccept #20833
rpc/validation: enable packages through testmempoolaccept #20833
Changes from all commits
42cf8b2
897e348
249f43f
b88d77a
578148d
2ef1879
cd9a11a
363e3d9
c9e1a26
ae8e6df
9ede34a
c4259f4
9ef643e
13650fe
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.
nit: I think a more descriptive behavior would be "if one transaction fails, remaining transactions are not submitted for validation". See document L1146 in src/validation.cpp "Exit early to avoid doing pointless work. Update the failed tx result; the rest are unfinished".
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.
maybe "if another transaction failed", because it's not necessarily an earlier transaction? (in the case where validation terminates early, the culprit could also be a later transaction)
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.
nit: space after maxfeerate
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.
Are you saying I should add a space after maxfeerate or?
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.
I meant after "maxfeerate." and before the next sentence, it should show when displaying the rpc help.
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.
nit: the check on
MAX_PACKAGE_COUNT
is duplicated intoAcceptMultipleTransactions
. I can understand the rational of not taking thecs_main
lock but I've a preference to keep all package policy checks in one place. That would also avoid linkingpackages.h
inrawtransaction.cpp
(assuming we hardcode the package limit in RPC doc).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.
I think of this as a distinct check, actually. We defined the
testmempoolaccept
API to be "maximum 25 transactions" and will return a JSONRPC error here because the user violated our API, whereasAcceptMultipleTransactions()
is applying package policies.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.
nit: transaction result
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.
fees while we iterate, keave->leave