Api improvements #131
Api improvements #131
Conversation
We already use a lower boundary in sat to perform a swap due to economical reasons. We want to be able to return this value via the api, so this commit moves this value to the policy where it belongs. The MinSwapAmountMsat is now in msat, set to a default of 100000000 msat and can NOT be overridden in the policy file. We might change this in the future. This might change at some point in the future.
We missed some mutex locks on the getters.
GLightning returned the struct in camel case but we want to use snake case for the returned values / structs.
We want a cleaner toolkit for developing and an easy way to setup a linear network of 2 cln nodes.
We want a cleaner toolkit for developing and an easy way to setup a linear network of 2 lnd nodes.
We want to return the same data for the listswap calls on both peerswapd for lnd and the cln peerswap plugin. We use the protos and the generated stubs to ensure the same return values on the listswaps, listactiveswaps, swapin and swapout commands.
We want a cleaner toolkit for developing and an easy way to setup a linear network of 2 lnd nodes.
We want to ensure that the string that is written to the policy file lists (suspicious_peers and allowlisted_peers) is not empty or not a valid hex representation of a pubkey.
We used to handle the global allowance of swaps non persistent in the swap service. It is better to have all these kind of constraints (this is a kill-switch) in one place. This also adds some methods to set and get the value.
When we generate the stubs from the proto the structs gets tagged with json:"omitempty". When we pass this message to a json marshaler for e.g. printing to stdout, the marshaler omits empty fields or fields that contain the default value for their type. What we want is to print all fields even if they are empty. This adds a custom marshaler that is configured to emit unpopulated values.
We want to return the policy struct. This uses the generated struct from the peerswaprpc protos.
We now use the policy instead of the inmemory map to determine if we can start a new swap or not. In the case of a incoming swap request from a peer, this check is moved to the CheckRequestWrapperAction as we also want to send our peer a cancel message if we do not allow for swaps atm. Prior this commit the check was performed in the service methods and did not send a cancel message to the peer so that the peer could get stuck.
We want to enforce snake case.
|
Hi Peter, Please check my inline comments below for API improvement PR review:
[Shahana]: It is fixed.
[Shahana]: Throws error now:
[Shahana]: Mostly fixed except error object's field
[Shahana]: getSwap returns numeric values while 'listswaps', 'listactiveswaps' etc return strings for role & type both.
[Shahana]: Not available.
[Shahana]: Not available.
[Shahana]: Fixed, receiving it in reloadpolicy as
[Shahana]: Fixed, receiving it in reloadpolicy as
[Shahana]: Fixed with
[Shahana]: Not available.
[Shahana]: Fixed, does not get into the loop. It backs off for 30 seconds but does not reconnect even if lnd is up again within these 30 seconds. Peerswap restart required if lnd restarts.
[Shahana]: Fixed with error message
[Shahana]: Error from RPC call "Plugin terminated before replying to RPC call.", [CLN logs, first run, SwapIn without amount]: [CLN logs, second run, SwapIn without amount]: [CLN logs, third run, SwapOut without amount]:
|
Do not expect subscriptions to happen (too many variants of interfaces and we can't prioritize this.) The UI should periodically poll or have a manual refresh button.
Will not happen anytime soon due to priority. The amount of data is super tiny compared to how quickly other things like forwarding grow which are cases that need filtering.
ElementsProject/lightning#5294
It sounds nice to have. Please file in a new issue?
@nepet that reminds me If this is the case then I'm not concerned about the race condition potential of these commands because they are used almost exclusively by humans and have only an indirect effect on whether swaps can start or not. |
|
@wtogami You are correct, |
@ShahanaFarooqui fyi the |
This was a command for debugging purposes but somehow made it to the public api.
When writing data to the store fails, we return a failure. If this happens on swap creation we want the swap to be finished so that we can start a new swap
The enums role and type have been returned as int values. For a better human readability we now return them as string
I also observed this weirdness when ever I call peerswap too soon on startup. I will figure out how we can handle this. -> #137 |
Peerswapd is mostly terminating successfully with below message and needs manual restart after LND's restart.
|
|
Thanks for your review.
Fixed 86628e4
The internal representation of these fields is of type If it is okay for you to parse the string to a number I would not change anything about this, otherwise we would need to parse every struct that contains
These should not be related to this PR, could you file issues for this and provide more information?
As said before, this is an issue that arises when peerswap is called too early after startup (whil still waiting for lnd to sync) and is part of #137 Except for some minor issues that we wont address right now I'd say we are good to go and can build on top of this pr. |
We already format
This can be an issue for the UI. If LND's short channel Id is "119134283892916224" and Peerswapd is sending it as "108352:2:0", how will the UI map them together?
Created an issue here. I tried to provide as much information as I can but logs are not showing much details.
This issue is persistent and happening well after cln is synced, policy loaded and peerswap initialized etc. I tested it with commit id |
This is a wip PR to address some API improvements.
It so far contains:
reloadpolicyshowsmin_swap_amount_msatmin_swap_amountto policy (still hard-coded)addpeeraddsuspeerinputs (need to be a hex[66])allowswaprequests. This now persists in the policy. Alsoallowswaprequestsnow responds with the policy struct.listnodesrequest