-
-
Notifications
You must be signed in to change notification settings - Fork 355
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
Increase tx size limit up to 250kb #1457
Conversation
WalkthroughThis update increases the maximum transaction weight from 520,000 to 1,000,000 in the relay and mining policy. Adjustments are made across multiple files to reflect this new limit, ensuring consistent application in transaction validation and creation processes. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/policy/policy.h (1 hunks)
- src/spark/sparkwallet.cpp (1 hunks)
- src/wallet/wallet.cpp (1 hunks)
Additional comments not posted (2)
src/policy/policy.h (1)
28-28
: Increase inMAX_NEW_TX_WEIGHT
from 520000 to 1000000 to support larger transaction sizes.Please ensure that all components interacting with transaction sizes (like mining, transaction creation, and relay policies) are updated and tested to handle this new limit effectively.
Verification successful
The verification of the change in
MAX_NEW_TX_WEIGHT
from 520000 to 1000000 has been completed. The updated constant is used in several files (net_processing.cpp
,wallet.cpp
,lelantusjoinsplitbuilder.cpp
,sparkwallet.cpp
, andpolicy.cpp
), where it is primarily involved in conditional checks to ensure that transactions do not exceed the specified weight limit.The logic in these files appears to handle the new limit appropriately by either logging, rejecting, or throwing exceptions when the transaction weight exceeds
MAX_NEW_TX_WEIGHT
. Therefore, the change is consistent and should not introduce any immediate issues if the surrounding logic is correctly implemented.
net_processing.cpp
: Logs and ignores large orphan transactions.wallet.cpp
: Prevents the creation of oversized transactions.lelantusjoinsplitbuilder.cpp
: Throws an exception for oversized transactions.sparkwallet.cpp
: Prevents the creation and throws exceptions for oversized transactions.policy.cpp
: Rejects transactions that exceed the weight limit.Given the consistent handling across different parts of the codebase, the change seems to be well-integrated.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `MAX_NEW_TX_WEIGHT` to ensure they handle the new limit. # Search for usages of `MAX_NEW_TX_WEIGHT` and related logic in the codebase. Expect: Proper handling of the new limit. rg --type cpp --type h "MAX_NEW_TX_WEIGHT"Length of output: 620
Script:
#!/bin/bash # Extract relevant code snippets for each usage of `MAX_NEW_TX_WEIGHT` to review the logic in detail. # Extract snippet from net_processing.cpp echo "net_processing.cpp:" rg --context 5 --type cpp "MAX_NEW_TX_WEIGHT" src/net_processing.cpp # Extract snippet from wallet.cpp echo -e "\nwallet.cpp:" rg --context 5 --type cpp "MAX_NEW_TX_WEIGHT" src/wallet/wallet.cpp # Extract snippet from lelantusjoinsplitbuilder.cpp echo -e "\nlelantusjoinsplitbuilder.cpp:" rg --context 5 --type cpp "MAX_NEW_TX_WEIGHT" src/wallet/lelantusjoinsplitbuilder.cpp # Extract snippet from sparkwallet.cpp echo -e "\nsparkwallet.cpp:" rg --context 5 --type cpp "MAX_NEW_TX_WEIGHT" src/spark/sparkwallet.cpp # Extract snippet from policy.cpp echo -e "\npolicy.cpp:" rg --context 5 --type cpp "MAX_NEW_TX_WEIGHT" src/policy/policy.cppLength of output: 3508
src/spark/sparkwallet.cpp (1)
1026-1026
: Transaction weight limit check uses updated constant
The transaction weight limit check correctly uses the updatedMAX_NEW_TX_WEIGHT
constant. This change aligns with the PR's objective to increase the transaction size limit up to 259kb.
Currently, Firo’s max transaction limit size is 100 kB (inherited from Bitcoin) though some people are hitting this limit especially when dealing with many masternode or mining payouts. The error that pops up is 'Transaction is too large". The limit is there to prevent DDoS and ensure transactions can propagate properly. Core team proposed to up this to 250 kB to account for the larger size of Spark transactions and also since we already have a larger block size limit. This will assist with UTXO consolidations as well and UX. While technically not a hard fork since nodes will accept into the block, the transaction will not be relayed where the transaction is greater than 100 kB so this would be a mandatory upgrade. We sought community feedback and there were no objections: |
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/spark/sparkwallet.cpp (2 hunks)
- src/wallet/wallet.cpp (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/spark/sparkwallet.cpp
- src/wallet/wallet.cpp
No description provided.