feat: Allow restriction of number of inputs per tx #503
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Feel free to comment on design. Marked as draft because this new parameter is completely untested. Needs tests! |
Sword-Smith
requested review from
aszepieniec
and removed request for
aszepieniec and
jan-ferdinand
aszepieniec
reviewed
Mar 13, 2025
| @@ -346,13 +375,18 @@ impl Mempool { | |||
| } | |||
| } | |||
|
|
|||
| let mut events = vec![]; | |||
| // Check if tx conforms to mempool policy. | |||
There was a problem hiding this comment.
Suggested change
| // Check if tx conforms to mempool policy. | |
| // Check if tx conforms to mempool policy wrt # inputs |
Sword-Smith
force-pushed
the
thv/allow-restricting-num-inputs
branch
2 times, most recently
from
feac0b3 to
1f3d5eb
Compare
src/models/state/mempool.rs
Outdated
| @@ -152,6 +162,7 @@ impl Mempool { | |||
| pub fn new( | |||
| max_total_size: ByteSize, | |||
| max_num_transactions: Option<usize>, | |||
| max_num_inputs_per_tx: Option<usize>, | |||
There was a problem hiding this comment.
This interface is starting to get unwieldy. How about a builder instead? If you don't want to write one yourself, take a look at bon.
Allow the user to restrict the number of inputs per transaction to avoid having to deal with transactions that will likely never be mined, as they don't fit into a block. The current default limit is set quite low, to 28 inputs per transaction due to the big size of transaction inputs. This limit is intended to rise significantly in the future, but for now it should serve to prevent composers and transaction initiators from making transactions that cannot be mined. Both the wallet state and mempool state gets passed this value such that internally can enforce the correct policy.
Verify that the mempool respects the maximum number of inputs on a) insertion, b) returning a pair for proof-upgrading, and c) returning a list of transactions for inclusion in a block.
Sword-Smith
force-pushed
the
thv/allow-restricting-num-inputs
branch
from
1f3d5eb to
e38c78b
Compare
Sword-Smith
commented
Mar 16, 2025
src/models/state/mempool.rs
Outdated
| } | ||
|
|
||
| None | ||
| (0..self.len() - 1).find_map(|i| most_dense_pair_skip_first_n(self, i)) |
There was a problem hiding this comment.
I think the expression self.len() - 1 can overflow. Probably not good.
There was a problem hiding this comment.
Just shows that these getters need to be tested on an empty mempool. That would have caught this problem by producing a never-ending test case.
Add struct `MempoolSizePolicy` to contain policy for the maximum size of mempool, transactions in mempool, and transactions returned from mempool. All size restrictions on the mempool, max num transaction, max total size in memory, max number of inputs, are, with this commit, encapsulated in a struct. After the merger of hard-fork 1, #502, new rules for block validity wwill come into effetct: max num inputs/outputs/public announcements. Those rules can be enforced by the mempool by adding two new fields to this new struct.
Fix an overflow error that lead to a (practically) infinite loop.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Allow the user to restrict the number of inputs per transaction to avoid having to deal with transactions that will likely never be mined, as they don't fit into a block.
The current default limit is set quite low, to 28 inputs per transaction due to the big size of transaction inputs. This limit is intended to rise significantly in the future, but for now it should serve to prevent composers and transaction initiators from making transactions that cannot be mined.
Both the wallet state and mempool state gets passed this value such that they, internally, can enforce the correct policies.