-
Notifications
You must be signed in to change notification settings - Fork 64
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
base: master
Are you sure you want to change the base?
Conversation
/// It expects: | ||
/// - a placeholder | ||
/// - the current value | ||
pub fn new_disabled(placeholder: &str, value: &Value<String>) -> Self { |
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.
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?
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.
I had a couple of further thoughts:
- Unrelated to this PR (perhaps could be a separate commit), we could refactor the
new_trimmed()
andnew_amount_btc()
methods to callSelf::new()
with the modifiedon_change
parameter. - If these other methods also use
Option
, then you can use thenew_trimmed()
whether or not the replacement txid exists, and the only thing that will change is whether you passNone
orSome
to theon_change
parameter (rather than different methods being called in each case).
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.
We can have new_disabled for now. I prefer we keep the change small.
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. |
ping @edouardparis |
I just answered here, if it was the reason of your ping. |
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.
close #1138