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

gui: disable replacement fee input if tx is already replaced #1209

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pythcoiner
Copy link
Collaborator

this PR disable input on feerate TextInput if the tx already have been replaced, it's fine to do so because we still can RBF the new replacing tx.

image

close #1138

@pythcoiner pythcoiner self-assigned this Jul 19, 2024
@nondiremanuel nondiremanuel added the Nice to have If it's not completed in time for the current version, it can be postponed label Jul 19, 2024
@nondiremanuel nondiremanuel added this to the v7 - Liana milestone Jul 19, 2024
@darosior darosior requested a review from jp1ac4 July 19, 2024 12:38
/// It expects:
/// - a placeholder
/// - the current value
pub fn new_disabled(placeholder: &str, value: &Value<String>) -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps instead of adding this method, you could reuse the new() method by changing the on_change parameter to Option.

What do you think, @edouardparis?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I had a couple of further thoughts:

  1. Unrelated to this PR (perhaps could be a separate commit), we could refactor the new_trimmed() and new_amount_btc() methods to call Self::new() with the modified on_change parameter.
  2. If these other methods also use Option, then you can use the new_trimmed() whether or not the replacement txid exists, and the only thing that will change is whether you pass None or Some to the on_change parameter (rather than different methods being called in each case).

gui/src/app/view/transactions.rs Outdated Show resolved Hide resolved
@nondiremanuel
Copy link
Collaborator

Moving this and the relative issue to v8 since as per team discussion we won't be able to properly review and test it given the close deadline.

@nondiremanuel nondiremanuel modified the milestones: v7 - Liana, v8 - Liana Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Nice to have If it's not completed in time for the current version, it can be postponed
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

[GUI] RBF modal feerate is still editable once replacement PSBT generated
3 participants