MostroP2P / mostro

Lightning Network peer-to-peer exchange platform on Nostr
https://mostro.network
MIT License
172 stars 27 forks source link

Allowing the user to cancel a take #314

Closed dtonon closed 2 months ago

dtonon commented 2 months ago

As per subject, is there a technical reason because is not currently possible to cancel a take? If so, it could be useful to explain that on the payment page.

arkanoider commented 2 months ago

It should be not so complicated imo. Not seeing any issue about this. @grunch is the boss of protocol anyway, let's see what he thinks about.

grunch commented 2 months ago

As per subject, is there a technical reason because is not currently possible to cancel a take? If so, it could be useful to explain that on the payment page.

no reason, we should do that definitely

arkanoider commented 2 months ago

Give a look to #331 @grunch .

arkanoider commented 2 months ago

Removed #311 because it's not good. Lookin back to the code @grunch i think we have all the logic to cancel a take, it's already done in cancel.rs and I think it works with CLI. Now i cannot test it, maybe tonite. But is it just the fact that now works only from CLI:

These two calls do exactly the reset to pending state of the specific order.

@Catrya could you try a quick test with actual version?

arkanoider commented 2 months ago

As per subject, is there a technical reason because is not currently possible to cancel a take? If so, it could be useful to explain that on the payment page.

@dtonon could you confirm me that you only tested with web client? Probably the take cancel is yet functional, but only from CLI.

grunch commented 2 months ago

Removed #311 because it's not good. Lookin back to the code @grunch i think we have all the logic to cancel a take, it's already done in cancel.rs and I think it works with CLI. Now i cannot test it, maybe tonite. But is it just the fact that now works only from CLI:

* Here https://github.com/MostroP2P/mostro/blob/8588a2313a45ce9e542139e2d591970307d29b1a/src/app/cancel.rs#L59

* Here https://github.com/MostroP2P/mostro/blob/8588a2313a45ce9e542139e2d591970307d29b1a/src/app/cancel.rs#L65

These two calls do exactly the reset to pending state of the specific order.

@Catrya could you try a quick test with actual version?

Hi @arkanoider, understood, I'm out of home so I have less time to test, I'm prioritizing PR like the unsubscribe, @Catrya can you test this one please? 🥹

Catrya commented 2 months ago

@Catrya could you try a quick test with actual version? Hi @arkanoider, understood, I'm out of home so I have less time to test, I'm prioritizing PR like the unsubscribe, @Catrya can you test this one please? 🥹

Yes of course, yesterday I also tested it in the current version and I'm almost sure that the cancel a take didn't work, but in a few minutes I'll test it again and I'll tell you guys

arkanoider commented 2 months ago

@Catrya could you try a quick test with actual version? Hi @arkanoider, understood, I'm out of home so I have less time to test, I'm prioritizing PR like the unsubscribe, @Catrya can you test this one please? 🥹

Yes of course, yesterday I also tested it in the current version and I'm almost sure that the cancel a take didn't work, but in a few minutes I'll test it again and I'll tell you guys

Great! From the code it should reset order to Pending state. Try this sequence using mostro-cli:

Maybe I lost some memory on the code, but it should revert to pending state.

Catrya commented 2 months ago

Great! From the code it should reset order to Pending state. Try this sequence using mostro-cli:

  • Create a sell order.
  • Take it with takesell from cli.
  • Then from same user do a cancel command (not admin_cancel!).

Maybe I lost some memory on the code, but it should revert to pending state.

Done. Try to cancel a take Sell it does not cancel it, it does not even return any message to the taker, and the order remains in status: waiting-buyer-invoice

i'll test cancel a take buy

Catrya commented 2 months ago

Canceling a take buy does work, it returns the order from waiting-payment to pending status. @arkanoider

grunch commented 2 months ago

Canceling a take buy does work, it returns the order from waiting-payment to pending status. @arkanoider

Hi @dtonon can you confirm that you can cancel an waiting-payment order on mostro-cli ? if that is possible we should close this issue, if the issue is happening on mostro web please open an issue on mostro-web repository 😃

arkanoider commented 2 months ago

Great! From the code it should reset order to Pending state. Try this sequence using mostro-cli:

  • Create a sell order.
  • Take it with takesell from cli.
  • Then from same user do a cancel command (not admin_cancel!).

Maybe I lost some memory on the code, but it should revert to pending state.

Done. Try to cancel a take Sell it does not cancel it, it does not even return any message to the taker, and the order remains in status: waiting-buyer-invoice

i'll test cancel a take buy

@Catrya if you can could you copy and paste here the log on mostrod when you send the cancel command from mostro-cli?

arkanoider commented 2 months ago

@Catrya probably fixed the bug, try pulling latest commit. @grunch when you can give a look to the fix

arkanoider commented 2 months ago

Ok seems that #331 prove that this is yet possible, just web client does not manage take cancel after an order is taken.

grunch commented 2 months ago

closing this via #331

@dtonon we will release a new version with this fix next week, if you find this is not done or another bug just let us know, thank you

dtonon commented 2 months ago

@grunch

Hi @dtonon can you confirm that you can cancel an waiting-payment order on mostro-cli ?

Sorry I missed the mention. I just tried the web version, I actually opened the issue in the wrong repo.