lock inputs via strategies and unlock early due to payment anxiety#8
lock inputs via strategies and unlock early due to payment anxiety#8Mshehu5 wants to merge 1 commit intopayjoin:masterfrom
Conversation
425cef2 to
bb8c115
Compare
|
|
||
| /// Check if an input is locked by an active (non-expired) payjoin. | ||
| /// Inputs unlock early if the associated payment obligation deadline is approaching (payment anxiety). | ||
| fn is_input_locked_by_active_payjoin(&self, outpoint: &Outpoint) -> bool { |
There was a problem hiding this comment.
the method currently handles two distinct cases (2-party vs mp payjoins) in the same function body, which makes it harder to follow.
can we consider splitting these functions?
There was a problem hiding this comment.
Thanks for the Review !
I kept one function because lock checks share the same outpoint -> bulletin-board lookup I think is better if callers just use a single lock decision function (to stop from a senario where is used wrong)
I think i can improve the comments to make it easier to follow but for now I feel a single lock would be best
Do you feel this is okay?
There was a problem hiding this comment.
Sounds reasonable, this should prevent callers from accidentally checking only one. seems the tradeoff of being in complex function is worth it if the alternative is misused.
There was a problem hiding this comment.
Sounds reasonable, this should prevent callers from accidentally checking only one. seems the tradeoff of being in complex function is worth it if the alternative is misused.
| .info() | ||
| .unconfirmed_txos_in_payjoins | ||
| .contains_key(&o.outpoint()) | ||
| fn unspent_coins<'s>( |
There was a problem hiding this comment.
| fn unspent_coins<'s>( | |
| /// Spendable coins excluding confirmed spends and strategy-locked inputs (e.g. active payjoins). | |
| fn unspent_coins<'s>( |
655eff5 to
9c8ae45
Compare
| fn unspent_coins(&self) -> impl Iterator<Item = OutputHandle<'a>> + '_ { | ||
| self.potentially_spendable_txos().filter(|o| { | ||
| !self.info().unconfirmed_spends.contains(&o.outpoint()) | ||
| // TODO Startegies should inform which inputs can be spendable. |
There was a problem hiding this comment.
I realize this may have been misleading. Wallets should mark utxos as locked as secondary information (WalletInfo). Perhaps a better name for it could be: "used_utxos". This information should not live in the strategy struct
| fn is_payment_due_soon(&self, po_id: &PaymentObligationId) -> bool { | ||
| let po = po_id.with(self.sim).data(); | ||
| let time_to_deadline = po.deadline.0.saturating_sub(self.sim.current_timestep.0); | ||
| let anxiety_window = | ||
| (po.deadline.0.saturating_sub(po.reveal_time.0) / PAYMENT_ANXIETY_THRESHOLD).max(1); | ||
| time_to_deadline <= anxiety_window | ||
| } |
There was a problem hiding this comment.
This should be implemented in action.rs and should live with the rest of the scoring logic.
The way I think this should go
- Wallets should score handling an action with every utxo they have (even ones used in other payments that are not confirmed)
- If I use a UTXO that is locked but it means I handle a "more valuable" payment I should prefer to do that. The two costs should be evaluated against each other.
For example, Alice has only one UTXO and needs to pay Bob but there is ample time so she starts a Payjoin. UTXO a is locked in a payjoin that Alice started. She is waiting for Bob to respond to her. After a couple timesteps a higher priority tx shows up and Alice would prefer to spend the UTXO on that payment obligation. The cost function should inform the wallet that abandoning the payjoin is more valuable.
And this "every utxo" scaffolding will set us up nicely for subset sum portion of the cost function.
this PR aims to address two TODO in unspent coin wallet.rs L159-160
// TODO Startegies should inform which inputs can be spendable.
// TODO: these inputs should unlock if the payjoin is expired or the associated payment obligation is due soon (i.e payment anxiety)
coins are now locked when used by a strategy and unlock when expired or due to payment anxiety(payment obligation almost due) which is currently implemented as if less than 10% of your deadline window remains this will unlock the coins. This threshold can be changed as is just a value