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

Increase tx size limit up to 250kb #1457

Merged
merged 2 commits into from
Jun 18, 2024
Merged

Increase tx size limit up to 250kb #1457

merged 2 commits into from
Jun 18, 2024

Conversation

levonpetrosyan93
Copy link
Contributor

No description provided.

Copy link

coderabbitai bot commented Jun 17, 2024

Walkthrough

This 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

Files Change Summary
src/policy/policy.h Increased MAX_NEW_TX_WEIGHT constant from 520000 to 1000000.
src/spark/sparkwallet.cpp Updated condition check to use MAX_NEW_TX_WEIGHT instead of MAX_STANDARD_TX_WEIGHT.
src/wallet/wallet.cpp Modified CWallet::CreateTransaction method to compare transaction weight against MAX_NEW_TX_WEIGHT.

Poem

As weights increase, blocks do swell,
A million now, the ledger tells,
Transactions faster, limits extended,
Digital coins, rules amended.
Through lines of code, changes sweep,
In blockchain dreams, we rabbits leap.
🌟✨


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@levonpetrosyan93 levonpetrosyan93 changed the title Increase tx size limit up to 259kb Increase tx size limit up to 250kb Jun 17, 2024
Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between fc2e876 and 1450a72.

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 in MAX_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, and policy.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.cpp

Length 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 updated MAX_NEW_TX_WEIGHT constant. This change aligns with the PR's objective to increase the transaction size limit up to 259kb.

src/wallet/wallet.cpp Show resolved Hide resolved
@reubenyap
Copy link
Member

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:
https://forum.firo.org/t/feedback-on-upping-transaction-limit-size-from-100-kb-to-250-kb/3196

@reubenyap reubenyap added this to the Release 0.14.13.3 milestone Jun 17, 2024
@reubenyap reubenyap requested a review from psolstice June 17, 2024 09:53
Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 1450a72 and 1ff48e5.

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

@reubenyap reubenyap merged commit d23d3ce into master Jun 18, 2024
6 checks passed
@reubenyap reubenyap deleted the size_limit branch June 18, 2024 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants