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

Add CLI option to control feerate argument from mempool #1230

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

OBorce
Copy link
Contributor

@OBorce OBorce commented Sep 22, 2023

  • add CLI option in-top-x-mb argument to control how much to prioritize the
    transaction feerate so the transaction can get in the top x MB of the
    mempool
  • add functional test

@TheQuantumPhysicist TheQuantumPhysicist changed the title Add CLI option to control feerate argument from memppol Add CLI option to control feerate argument from mempool Sep 22, 2023
@@ -376,7 +376,8 @@ impl Backend {
.get_mut(&wallet_id)
.ok_or(BackendError::UnknownWalletIndex(wallet_id))?
.controller
.synced_controller(account_index)
// TODO: add option to select from GUI
.synced_controller(account_index, ControllerConfig { in_top_x_mb: 5 })
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, no magic numbers in the middle of the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added the constant at the top with a comment

Comment on lines 294 to 295
.expect("Subsystem call ok")
.expect("Subsystem call ok");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a very bad idea to have two expects, right after each other, and with the exact same message... basically if this error happens, it may be difficult to know where it really happened.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed the message

Base automatically changed from fix/token-issue-checks to master September 22, 2023 11:25
@OBorce OBorce marked this pull request as draft September 24, 2023 10:32
- add in-top-x-mb argument to control how much to prioritize the
  transaction feerate so the transaction can get in the top x MB of the
mempool
- add functional test
@OBorce OBorce force-pushed the feature/high-feerate-functional-test branch from e1804c0 to 40709c0 Compare September 25, 2023 10:38
@OBorce OBorce marked this pull request as ready for review September 25, 2023 10:38
@TheQuantumPhysicist TheQuantumPhysicist merged commit 648d378 into master Sep 25, 2023
23 checks passed
@TheQuantumPhysicist TheQuantumPhysicist deleted the feature/high-feerate-functional-test branch September 25, 2023 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants