-
-
Notifications
You must be signed in to change notification settings - Fork 354
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
Spark fee calculation fix #1424
Conversation
WalkthroughThe recent update brings a refined approach to the calculation of transaction sizes in the wallet software. It specifically adjusts the way Changes
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: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- src/qt/coincontroldialog.cpp (1 hunks)
- src/spark/sparkwallet.cpp (1 hunks)
Additional comments not posted (5)
src/qt/coincontroldialog.cpp (1)
530-532
: The adjustments to thenBytes
calculation for Spark transactions introduce specific constants and multipliers for different transaction components. Ensure these values accurately reflect the transaction structure and sizes for Spark and other proofs. It's crucial to validate these constants against the actual transaction sizes to prevent underestimating or overestimating transaction fees.Consider adding comments detailing the source or rationale behind these constants and multipliers for future maintainability and clarity. This will help others understand the basis of these calculations and make it easier to update them if transaction structures change.
src/spark/sparkwallet.cpp (4)
1698-1700
: The calculation for the transaction size seems to be hardcoded with specific values for grootle proofs, private outputs, and transaction parts. This approach might not be flexible or accurate for future changes or different transaction types.Consider implementing a more dynamic way to calculate transaction sizes based on the actual components of the transaction. This could involve calculating sizes based on the serialized sizes of each component or using predefined constants that can be easily updated.
1698-1700
: The loop for fee estimation inSelectSparkCoins
recalculates the required amount and selects coins until the estimated fee stabilizes. However, this approach might lead to unnecessary iterations if the fee estimation does not change significantly between iterations.Consider adding a condition to break out of the loop if the difference between the new fee and the previous fee is below a certain threshold. This could improve efficiency by avoiding unnecessary iterations.
1698-1700
: The methodGetAvailableSparkCoins
correctly filters out coins based on coin control selection and locked coins. However, the readability of the lambda function used for filtering could be improved.Consider extracting the lambda function into a separate, named function or adding comments to explain the filtering criteria more clearly. This could improve the readability and maintainability of the code.
1698-1700
: The methodCreateSparkSpendTransaction
contains complex logic for creating a Spark spend transaction. The error handling in this method could be improved to provide more detailed information about the failure reasons.Consider adding more specific error messages or exceptions for different failure scenarios, such as insufficient funds, transaction size limits, or invalid inputs. This could help in diagnosing issues more effectively.
Summary by CodeRabbit