-
Notifications
You must be signed in to change notification settings - Fork 1
Payjoin v4 #3
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
base: master
Are you sure you want to change the base?
Payjoin v4 #3
Conversation
# Conflicts: # liana-gui/src/app/message.rs # liana-gui/src/app/state/receive.rs # liana-gui/src/app/view/receive.rs # liana-gui/src/daemon/client/mod.rs # liana-gui/src/daemon/embedded.rs # liana-gui/src/daemon/mod.rs # liana-gui/src/services/connect/client/backend/mod.rs
This also updated the receiver to derive input weight when constructing inputs
Just a small commit to begin cleaning up some unhandled unwraps and begin to move away from the reliance on the session metadata.
This reverts commit 357138b.
| SendPayjoin(Result<(), Error>), | ||
| PayjoinInitiated(Result<String, Error>), |
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.
Could have better names to differenciate between these
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 think the former could be sentPayjoin
| LoadWallet(Wallet), | ||
| Info(Result<GetInfoResult, Error>), | ||
| ReceiveAddress(Result<(Address, ChildNumber), Error>), | ||
| ReceiveAddress(Result<(Address, ChildNumber, Option<Url>), Error>), |
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.
If bip21 is get shipped as its own PR. We may want to use bitcoin_uri for this type instead
liana-gui/src/app/state/psbt.rs
Outdated
|
|
||
| #[derive(Default)] | ||
| pub struct SendPayjoinModal { | ||
| _error: Option<Error>, |
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.
If errors cannot occur lets remove this
| } | ||
| Task::none() | ||
| } | ||
| Message::View(view::Message::ShowBip21QrCode(i)) => { |
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.
Feature needed: add amount field. Currently the bip21 is missing this and our refrence implementation will complain
| self.label.value = label; | ||
| } | ||
| view::CreateSpendMessage::Bip21Edited(_, bip21) => { | ||
| log::info!("bip21: {}", bip21); |
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.
At this point we can clean up these debug logs
| }) | ||
| } | ||
|
|
||
| async fn receive_payjoin(&self) -> Result<GetAddressResult, DaemonError> { |
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.
Should implement these
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.
Although im unsure how to setup Liana so we use this daemon
| // // TODO: use a stronger type like bitcoin_uri | ||
| // pub bip21: String, |
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.
Can remove
| } | ||
|
|
||
| pub fn qr_modal<'a>(qr: &'a qr_code::Data, address: &'a String) -> Element<'a, Message> { | ||
| let max_width = if address.len() > 64 { 600 } else { 400 }; |
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 think we are re-using this modal for the bip21. In which case we should rename the address to param to something more general
| button::secondary(None, "Send Payjoin") | ||
| .on_press(Message::Spend(SpendTxMessage::SendPayjoin)) | ||
| .width(Length::Fixed(150.0)), | ||
| ) |
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.
Triple else indicates a better way to write this
liana did not have a way to add coins from external wallets to the db, this adds those coins to the db and forces is_from_self to be false and sets the wallet_id to 2 which is never the users wallet as it is hardcoded to 1.
We have a few stray clippy warnings causing our linter to fail in the CI.
This missing method in the dummy test utils was again causing some more clippy faliures.
The for loop in our receiver session was causing any error in a session to bubble up and completely stop the loop meaning follow-up sessions could not proceed forward.
No description provided.