RPC server for API Interface #1276
Conversation
|
@delta1 @yamabiiko Please give as much feedback and scrutiny as possible. The API is really important and we need to make sure that it's stable enough to be used by a lot of GUIs etc. in the future. I'll also give feedback in the next days and try to be as thorough as possible. |
|
Will review when I can in the next few days. @yamabiiko is there a reason for duplicated code instead of importing from other modules? |
Could you point out the specific examples? |
|
Seems like the ubuntu-latest runner is full. Maybe this can be helpful |
testing on my fork here: https://github.com/delta1/xmr-btc-swap/actions/runs/7872376768/job/21477456268 |
|
Seems like all the main concerns have been addressed and tests are passing successfully. Are we waiting for a final review or some other process I can contribute to? |
indeed sorry i am busy with some personal issues but should have a final review/approval next week @binarybaron do you have a moment to review, and test this? |
| if self.meets_target(target) { | ||
| 0 | ||
| } else { | ||
| target.into() - self.confirmations() |
There was a problem hiding this comment.
not to self: this can't be negative because !meets_target implies confirmations < target
swap/src/api/request.rs
Outdated
| monero_receive_address, | ||
| swap_id, | ||
| } => { | ||
| context.swap_lock.acquire_swap_lock(swap_id).await?; |
There was a problem hiding this comment.
@binarybaron was this addressed?
Co-authored-by: Byron Hambly <[email protected]>
Co-authored-by: Byron Hambly <[email protected]>
Can you expand? Don't know what you mean |
Yes, the swap lock release logic was addressed by c7f411c and improved in the next few commits |
|
I'll do a few last tests and then this can be merged. @yamabiiko Can you give me an XMR address to send the Bounty to? I want to thank you for all your work and I apologize for the long time it took to get this PR merged. This PR really is of immense value. It will dramatically improve the usability of this project. I really mean it. |
and resolve conflicts
Sounds good, I just resolved the conflicts. |
|
@binarybaron merging this now, any issues can be fixed in a follow-up. @yamabiiko congrats! thank you for the contribution 🙏 |
Thanks for merging :))) we're missing a changelog sadly but its no big deal |
|
yes definitely need a changelog entry. |
Opening this PR so it can start getting reviewed and possibly get some help.
As discussed in #1185, this is quite a substantial rework which adds the possibility of running an RPC server and unifies the RPC methods and CLI commands under an internal API.
To be done before we can get this merged:
swap/tests/rpc.rs: right now the test runs the RPC server but the functionality cannot be tested as the Bitcoin wallet does not have any balance and the swap database is empty (so right now it's using some placeholder BTC/XMR addresses and swap ids and it will always fail).If someone could help with writing proper tests (or pointing out at some examples in the code to how swaps and transactions can be simulated/tested) it would be appreciated.