robosats
robosats copied to clipboard
RoboSats
POST /api/order cancel should include current status
Describe the bug
It is not a bug, it is a security improvement (for the client).
When a POST requests is make to /api/order with action cancel, the order is cancelled. Depending on the current status of the order, a penalty may be inflicted on the canceller. The point is that the client can not be 100% percent sure that the current status of the order is the one it think it is. A public order may for example have been taken after the last request of the client to get the current status.
By adding a new parameter in the requests, (current_status for example), the client specifes the status in which it wants to cancel the offer. The server then proceeds in cancelling the offer just if the current status is the one specified in the request, if not it simply returns an error saying that current status is not the correct one.
To Reproduce
Make a POST requsts at /api/order with action cancel
Expected behavior Prevent the client from cancelling an offer in the wrong status
This is a great solution to the problem :+1: It is rare, but it can happen in our official client that a maker cancels the order while still public and by the time the request hits, a taker has shown up, therefore making the maker lose his bond without any previous notification and no recourse.
Implementing this fix on the backend and client is rewarded with :zap: 100K Sats :zap: (60K Sats for a PR with a backend only fix, and 30K Sats for a PR with client only fix)
@Reckless-Satoshi I think I can start working on this, could you assign me?
Hello @aftermath2, I have made a PR that implements this fix on the backend. I am not familiar with frontend sutff, you can work on the frontend if you want.
@jerryfletcher21 I've already made the changes to the backend and frontend (PR), sorry I didn't know you were working on it.
@aftermath2 ah that is a bit unfortunate that we both worked at the backed. Let's wait for @Reckless-Satoshi's feedbacks and then we may do a a single pull requests.
Haha, okay this is a failure guys! :smile: :rofl:
To not over-complicate things, we'll merge #1326 but there is a few things from #1325 that make sense to incorporate. More details on #1325 #1326 reviews.
@Reckless-Satoshi Just rebased to main to pull the latest changes, resolved a conflict and ran the tests. I think the issue is ready to review, let me know if there are further changes needed.
@KoalaSat I would like to work on this, Could you please assign me this one?
@KoalaSat How much space and internet bandwidth does the full-stack development setup require? It seems to be taking a long time to complete.
@KoalaSat @MrImmortal09 I think this is already done, and waiting for being merged in #1326
Oh! I didn't noticed @jerryfletcher21 sorry I'm still trying to do a bug clean up of active PRs 🙂
Sorry for the confusion @MrImmortal09
Regarding the PR, I did notice it but haven’t reviewed the code before. I followed up by asking @aftermath2 if he was working on it, but I haven’t received a response. So, it’s unclear whether he’s available or AFK.
@MrImmortal09 I did work on it, but now the PR is just pending a review to get merged. There shouldn't be many more changes required.
@MrImmortal09 I did work on it, but now the PR is just pending a review to get merged. There shouldn't be many more changes required.
Thank you! I'll review it ASAP
Merging! Please @aftermath2 submit an invoice with a long expiration time for a DevFund reward of 100K Sats.
@KoalaSat Thanks!
lnbc1m1pn7jnt0sp56996m6sjfwwh4l9gllryz3qjwc96ezpt0gd4hukhctkurp2kedqspp5dlpsuz4wv6cjvnhqegnhfukw7fm4gw85y0l242ndtfsrye5pk3mshp5uwcvgs5clswpfxhm7nyfjmaeysn6us0yvjdexn9yjkv3k7zjhp2sxq9z0rgqcqpnrzjq0gxwkzc8w6323m55m4jyxcjwmy7stt9hwkwe2qxmy8zpsgg7jcuwrt3xsqqefsqqyqqqqlgqqqqmtqqjq9qxpqysgqmuy5604an6t5guvy24xpm25uzfgh5xd5rql28npq0lu6lu806pz52ddnhvw8w6c2taya0p6jnajtskjrtga9xyv5vlrrn9x64mmwrfqpwgk6c8
@KoalaSat Thanks!
lnbc1m1pn7jnt0sp56996m6sjfwwh4l9gllryz3qjwc96ezpt0gd4hukhctkurp2kedqspp5dlpsuz4wv6cjvnhqegnhfukw7fm4gw85y0l242ndtfsrye5pk3mshp5uwcvgs5clswpfxhm7nyfjmaeysn6us0yvjdexn9yjkv3k7zjhp2sxq9z0rgqcqpnrzjq0gxwkzc8w6323m55m4jyxcjwmy7stt9hwkwe2qxmy8zpsgg7jcuwrt3xsqqefsqqyqqqqlgqqqqmtqqjq9qxpqysgqmuy5604an6t5guvy24xpm25uzfgh5xd5rql28npq0lu6lu806pz52ddnhvw8w6c2taya0p6jnajtskjrtga9xyv5vlrrn9x64mmwrfqpwgk6c8
68b5345f7753d54a6ab66c4105d4b979c87b826c2df39072963c1b040a5e7a51