fix(rpc): no success if no channels to close (#1689) #1942
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
we can just add check for connext currencies - |
Would fix it, but certainly not beautiful. If then we should have the same for lnd. Thoughts? @sangaman |
sangaman
requested changes
Oct 21, 2020
rsercano
force-pushed
the
fix/closechannel-prints-success-for-no-channel
branch
from
71b870b to
18234dc
Compare
|
@sangaman added error code to the @raladev I'm not sure about showing alias, this requires me to pick Pool.ts into LndClient.ts which would necessarily be a bad practice just to show a log if there's no other way @sangaman @sangaman could you please clarify @kilrau @raladev's point about connext channel balance? |
sangaman
force-pushed
the
fix/closechannel-prints-success-for-no-channel
branch
from
2f95e8c to
1981357
Compare
|
Let me know if anything else has to be done for this please @sangaman |
Yep that needs to be clarified... |
|
|
@raladev Could you please try this again? The |
|
works, but:
|
rsercano
force-pushed
the
fix/closechannel-prints-success-for-no-channel
branch
from
f7a2eb7 to
029eae8
Compare
|
added @raladev |
raladev
previously approved these changes
Oct 28, 2020
|
Needs a rebase. |
raladev
previously approved these changes
Oct 30, 2020
sangaman
previously approved these changes
Nov 1, 2020
sangaman
dismissed
their stale review
looks like some changes were lost during rebase
This ensures we don't return a successful response and print success in the cli when attempting to close channel(s) that don't exist. Closes #1689. Co-Authored-By: rsercano <ozdemirsercan27@gmail.com>
sangaman
force-pushed
the
fix/closechannel-prints-success-for-no-channel
branch
from
995626d to
b279e72
Compare
sangaman
previously approved these changes
Nov 2, 2020
|
Hm... could you attach the xud logs? It looks like the call to get ETH/USDT balance are failing. Maybe that's simply what happens when there is no balance or no channel for Connext and so we can just handle that error as |
|
Let me know, if I can do anything on this please |
Same error as was before, this case was fixed by @rsercano previously, but it seems we lose the fix during rebase xud: Connext: |
|
I don't think we lost the changes tho, it still shows changes are there, @sangaman's addition is as simple as checking swap client type in the |
|
checked once more time, same result, but im sure that it was fine some time ago |
rsercano
force-pushed
the
fix/closechannel-prints-success-for-no-channel
branch
from
10eb823 to
70d6678
Compare
raladev
approved these changes
Nov 6, 2020
|
Interesting, the only difference between the previous commit and the current one is at line 778 in ConnextClient.ts - if (Number(amount) === 0) {
+ if (amount === 0) {
return []; // there is nothing to withdraw and no tx to return
}It didn't have the |
ghost
approved these changes
Nov 6, 2020
sangaman
changed the title
raladev
added a commit
that referenced
this pull request
Nov 19, 2020
* feat: removeorder output changed to a more meaningful message (#1526) * fix(p2p): don't reconnect peers when pool closed (#1965) This ensures that we don't attempt to reconnect to peers that have disconnected from us after we have started closing the p2p pool. This may help prevent scenarios where we unintentionally attempt to reconnect to peers after shutting down xud. > Should be tested against [#1668 (comment)](#1668 (comment)) @raladev re-connection after shutdown is disappeared, but my xud still can not be gracefully terminated, it waits something: ``` 28/10/2020 05:17:43.164 [CONNEXT] trace: sending request to /balance/0x69C3d485623bA3f382Fc0FB6756c4574d43C1618 ^C28/10/2020 05:17:44.087 [GLOBAL] info: XUD is shutting down 28/10/2020 05:17:44.088 [LND-BTC] info: new status: Disconnected 28/10/2020 05:17:44.089 [LND-LTC] info: new status: Disconnected 28/10/2020 05:17:44.090 [CONNEXT] info: new status: Disconnected 28/10/2020 05:17:44.093 [P2P] debug: Peer 03ece33a30db1dbce4b62fa96a5e9541138a24997ef5672eebed2d332270e39542 (OzoneYellow): closing socket. reason: Shutdown 28/10/2020 05:17:44.095 [HTTP] info: http server has closed 28/10/2020 05:17:44.096 [RPC] info: gRPC server completed shutdown 28/10/2020 05:17:44.097 [P2P] trace: Sent Disconnecting packet to 03ece33a30db1dbce4b62fa96a5e9541138a24997ef5672eebed2d332270e39542 (OzoneYellow): "{\"body\":{\"reason\":9},\"header\":{\"id\":\"95133be0-1917-11eb-b75b-73d0f0278756\"}}" 28/10/2020 05:17:44.109 [ORDERBOOK] debug: removed all orders for peer 03ece33a30db1dbce4b62fa96a5e9541138a24997ef5672eebed2d332270e39542 (OzoneYellow) 28/10/2020 05:17:44.118 [GLOBAL] info: XUD shutdown gracefully ``` * feat(lnd): change gRPC client options * fix(connext): not enough balance for closechannel (#1963) This introduces better error handling for Connext when using `closeChannel` to remove funds from a Connext channel and specifying an amount to remove that is greater than the available balance. * feat: reserved capacity checks on PlaceOrder (#1949) This rejects orders that would put our total reserved balance over our total capacity for either the outbound or inbound currency. The sum of the inbound & outbound amounts for a newly placed order are added to the amounts reserved by open orders, and if either of these amounts exceed the corresponding capacity then the request to place the order is rejected. An exception to this are inbound limits for Connext currencies, since we have the ability to dynamically request additional inbound collateral via our "lazy collateral" approach. It is still possible for market orders to cause our open orders to exceed our capacity. This is a difficult problem to avoid entirely, as the price that market orders will execute at is unknown until the execution is complete. Even if we simulate the matching routine, we won't know which matches will succeed until we attempt a swap. Instead, we generously assume that market orders will execute at the best quoted price for purposes of these capacity checks. For users that simultaneously place limit orders and market orders for the same currencies, it should be made clear that market orders may use up their available balance needed for their limit orders to succeed. Closes #1947. * fix(cli): openchannel assertion error for string amount (#1950) Fixes #1643. * feat(swapclient): auto init wallets on xud unlock (#1973) This adds a new feature to xud to automatically attempt to create a wallet for any new swap client configured after an xud node has been created. Effectively this only changes the behavior for lnd clients, as this is already the existing behavior for Connext. The process for initializing has now been standardized instead of the ad hoc approach used previously. If xud tries to unlock an lnd node and gets an error message indicating that the wallet has not been created, then it will generate a client & currency specific seed mnemonic using seedutil and call `InitWallet` with that seed and the existing xud password, such that the wallet funds and node identity for the new lnd client can be unlocked and restored along with the rest of lnd. Closes #1929. * feat(rpc): runtime addcurrency for lnd & connext (#1746) Co-authored-by: Le Premier Homme <interjoint1@gmail.com> * refactor(rpc): rename reserved TradingLimits fields This renames the `reservedOutbound` and `reservedInbound` fields on the `TradingLimits` call to `reservedSell` and `reservedBuy` respectively. * fix(rpc): no success if no channels to close (#1689) (#1942) Co-authored-by: rsercano <ozdemirsercan27@gmail.com> Co-authored-by: Daniel McNally <mcnallydp@gmail.com> * fix: tls certificate check on startup (#1510) * fix: alias missing in streamorders (#1725) (#1962) * fix(lnd): handling hold invoice check errors (#1969) This adds better error handling for when the test calls to verify lnd hold invoices are available fail due to connectivity reasons. Previously any error that occurred at this step would cause us to set lnd's status to `NoHoldInvoiceSupport` including connection issues. There was also a bug that caused us to try to set the status to connected even when a hold invoice status check failed. This could result in the unusual behavior of status going to `Disconnected` upon a call failing due to the grpc `UNAVAILABLE` error status, then being set to `NoHoldInvoiceSupport` and then to `ConnectionVerified`. Now we only set `NoHoldInvoiceSupport` when the test calls fail for a reason other than `UNAVAILABLE`, and we only set the status to `ConnectionVerified` when the hold invoice calls succeed. Closes #1968. * feat(lnd): SendPaymentV2 This migrates the call we use to send payments with lnd from the deprecated `SendPaymentSync` to `SendPaymentV2` which allows for multi path payments, among other improvements. As part of this change, the lnd proto files have been updated to their v0.11.x versions and the version of lnd used in simulation tests has been updated to v0.11.1 as well. Closes #1590. * Revert "feat: removeorder output changed to a more meaningful message (#1526)" * fix: use regtest instead of regnet arg This fixes a bug where the xud flag to set the network was incorrectly configured as `regnet` when it should be `regtest` to match what xud expects. * fix(lnd): don't calculate negative capacities This fixes a bug in the logic for calculating the inbound & outbound amount/capacity totals. We subtract the channel reserve amounts from the balances when determining how large a payment the channel could support, however we should not end up with a negative number. * feat: new grpc call for subscribring alerts such as low balance (#864) * fix: changes removeorder output to a more meaningful message (#1526) (#1986) Co-authored-by: rsercano <ozdemirsercan27@gmail.com> Co-authored-by: Daniel McNally <mcnallydp@gmail.com> Co-authored-by: Karl Ranna <karl@karlranna.com> Co-authored-by: Le Premier Homme <interjoint1@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves #1689 for LND but I'm not sure what to do for connext since it doesn't show currently available channels and we directly call
withdraw