search

ExchangeUnion / xud

Exchange Union Daemon 🔁 ⚡️
https://exchangeunion.com
GNU Affero General Public License v3.0
115 stars 44 forks source link

fix(rpc): no success if no channels to close (#1689) #1942

Closed rsercano closed 4 years ago

rsercano commented 4 years ago

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

raladev commented 4 years ago

we can just add check for connext currencies - throw errorCodes.NO_CHANNELS_TO_CLOSE if channel balance = 0

simnet > closechannel ETH
Error: 2 UNKNOWN: connext server error 500: Internal Server Error

Bd39EC9EDDB315B45 request in 60 ms
[1603201176510] ERROR (70 on connext): Error: Error: Cannot install an app with zero valued deposits for both initiator and responder.
    at commonAppProposalValidation (/app/node_modules/@connext/apps/dist/index.js:66:15)
    at sharedProposalMiddleware (/app/node_modules/@connext/apps/dist/index.js:432:12)
    at proposeMiddleware (/app/node_modules/@connext/apps/dist/index.js:491:5)
    at validationMiddleware (/app/node_modules/@connext/apps/dist/index.js:438:23)
    at /app/node_modules/@connext/client/dist/channelProvider.js:60:15
    at Generator.next (<anonymous>)
    at /app/node_modules/@connext/client/dist/channelProvider.js:8:71
    at new Promise (<anonymous>)
    at __awaiter (/app/node_modules/@connext/client/dist/channelProvider.js:4:12)
    at validationMiddleware (/app/node_modules/@connext/client/dist/channelProvider.js:59:67)
    at /app/node_modules/@connext/cf-core/dist/protocol/propose.js:55:23
    at Generator.next (<anonymous>)
    at resume (/app/node_modules/@connext/cf-core/dist/protocol/propose.js:8:44)
    at /app/node_modules/@connext/cf-core/dist/protocol/propose.js:7:121
    at new Promise (<anonymous>)
    at Object.i.<computed> [as next] (/app/node_modules/@connext/cf-core/dist/protocol/propose.js:7:63)
    at ProtocolRunner.runProtocol (/app/node_modules/@connext/cf-core/dist/machine/protocol-runner.js:84:39)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async ProtocolRunner.initiateProtocol (/app/node_modules/@connext/cf-core/dist/machine/protocol-runner.js:44:33)
    Error: Error: Error: Cannot install an app with zero valued deposits for both initiator and responder.
        at commonAppProposalValidation (/app/node_modules/@connext/apps/dist/index.js:66:15)
        at sharedProposalMiddleware (/app/node_modules/@connext/apps/dist/index.js:432:12)
        at proposeMiddleware (/app/node_modules/@connext/apps/dist/index.js:491:5)
        at validationMiddleware (/app/node_modules/@connext/apps/dist/index.js:438:23)
        at /app/node_modules/@connext/client/dist/channelProvider.js:60:15
        at Generator.next (<anonymous>)
        at /app/node_modules/@connext/client/dist/channelProvider.js:8:71
        at new Promise (<anonymous>)
        at __awaiter (/app/node_modules/@connext/client/dist/channelProvider.js:4:12)
        at validationMiddleware (/app/node_modules/@connext/client/dist/channelProvider.js:59:67)
        at /app/node_modules/@connext/cf-core/dist/protocol/propose.js:55:23
        at Generator.next (<anonymous>)
        at resume (/app/node_modules/@connext/cf-core/dist/protocol/propose.js:8:44)
        at /app/node_modules/@connext/cf-core/dist/protocol/propose.js:7:121
        at new Promise (<anonymous>)
        at Object.i.<computed> [as next] (/app/node_modules/@connext/cf-core/dist/protocol/propose.js:7:63)
        at ProtocolRunner.runProtocol (/app/node_modules/@connext/cf-core/dist/machine/protocol-runner.js:84:39)
        at runMicrotasks (<anonymous>)
        at processTicksAndRejections (internal/process/task_queues.js:97:5)
        at async ProtocolRunner.initiateProtocol (/app/node_modules/@connext/cf-core/dist/machine/protocol-runner.js:44:33)
        at WithdrawalController.<anonymous> (/app/node_modules/@connext/client/dist/controllers/WithdrawalController.js:83:23)
        at Generator.throw (<anonymous>)
        at rejected (/app/node_modules/@connext/client/dist/controllers/WithdrawalController.js:6:65)
        at runMicrotasks (<anonymous>)
        at processTicksAndRejections (internal/process/task_queues.js:97:5)
[1603201176510] INFO  (70 on connext): request completed
    url: "/withdraw"
    statusCode: 500
    reqId: 4747
[1603201178439] INFO  (70 on
kilrau commented 4 years ago

we can just add check for connext currencies - throw errorCodes.NO_CHANNELS_TO_CLOSE if channel balance = 0

Would fix it, but certainly not beautiful. If then we should have the same for lnd. Thoughts? @sangaman

rsercano commented 4 years ago

@sangaman added error code to the getGrpcErrorCode, thanks for pointing.

@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?

raladev commented 4 years ago

@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

Oke, understood

rsercano commented 4 years ago

Let me know if anything else has to be done for this please @sangaman

kilrau commented 4 years ago

Let me know if anything else has to be done for this please @sangaman

Yep that needs to be clarified...

raladev commented 4 years ago 
27/10/2020 06:14:07.176 [RPC] trace: received call /xudrpc.Xud/CloseChannel
27/10/2020 06:14:07.177 [CONNEXT] trace: sending request to /balance/0x0000000000000000000000000000000000000000
27/10/2020 06:14:07.395 [CONNEXT] trace: sending request to /withdraw: {"recipient":"","amount":"0","assetId":"0x0000000000000000000000000000000000000000"}
27/10/2020 06:14:07.784 [CONNEXT] error: connext server error 500: Internal Server Error
27/10/2020 06:14:07.789 [RPC] error: call /xudrpc.Xud/CloseChannel errored with code 2: connext server error 500: Internal Server Error
sangaman commented 4 years ago

@raladev Could you please try this again? The amount was sometimes being a string so it would fail the === 0 number comparison.

raladev commented 4 years ago

works, but:

native@ubuntu:~/xud/bin$ ./xucli closechannel LTC OzoneYellow Error: 9 FAILED_PRECONDITION: no channels found to close for OzoneYellow

rsercano commented 4 years ago

added @raladev

ghost commented 4 years ago

Needs a rebase.

raladev commented 4 years ago

closechannel USDT throwed Error: 9 FAILED_PRECONDITION: no channels found to close before instead of 500 Err

simnet > closechannel BTC
Error: 3 INVALID_ARGUMENT: operation failed due to a missing remote identifier
simnet > closechannel BTC OzoneYelow
Error: 5 NOT_FOUND: node with alias OzoneYelow not found
simnet > closechannel BTC OzoneYellow
{
  "transactionIdsList": [
    "3e5eec5e9bb3c12c2f0f9d3890fc268a0aa3632bbaf1fd2f60f61d8bcef31bd9"
  ]
}
simnet > closechannel BTC OzoneYellow
Error: 9 FAILED_PRECONDITION: no channels found to close for OzoneYellow
simnet > closechannel ETH --amount 1
{
  "transactionIdsList": [
    "0xb0dc8025f777e477ebb35f36517885f855977576f4b206f1541108a8edfe2771"
  ]
}
simnet > closechannel ETH --amount 9
{
  "transactionIdsList": [
    "0x9d14d60da493f0740a4a0a521e23de4a4d93ecc6a1c0b47675e5d87c09284ef1"
  ]
}
simnet > closechannel ETH --amount 9
Error: 9 FAILED_PRECONDITION: insufficient balance to perform request
simnet > closechannel ETH
Error: 2 UNKNOWN: connext server error 500: Internal Server Error
simnet > closechannel ETH
Error: 2 UNKNOWN: connext server error 500: Internal Server Error
simnet > closechannel ETH
Error: 2 UNKNOWN: connext server error 500: Internal Server Error
simnet > closechannel USDT
{
  "transactionIdsList": [
    "0x7fa666342dbc6a0008424adab0ce7754318716c70014fd8c34ef7e0871081adc"
  ]
}
simnet > closechannel USDT
Error: 2 UNKNOWN: connext server error 500: Internal Server Error
simnet >
sangaman commented 4 years ago

Above

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 no channels found to close, unfortunately the 500 connext server error isn't very helpful.

rsercano commented 4 years ago

Let me know, if I can do anything on this please

raladev commented 4 years ago

Above

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 no channels found to close, unfortunately the 500 connext server error isn't very helpful.

Same error as was before, this case was fixed by @rsercano previously, but it seems we lose the fix during rebase

xud:

03/11/2020 11:30:21.325 [CONNEXT] error: connext server error 500: Internal Server Error
03/11/2020 11:30:21.342 [RPC] error: call /xudrpc.Xud/CloseChannel errored with code 2: connext server error 500: Internal Server Error

Connext:

2020-11-03T11:30:21.318Z [CF-ProposeInstall] Acquired locks 0x227Ef60B59F7B82EeB05aC4042B7503292A1dD25 in 194 ms
[1604403021320] ERROR (70 on connext): Error: Error: Cannot install an app with zero valued deposits for both initiator and responder.
    at commonAppProposalValidation (/app/node_modules/@connext/apps/dist/index.js:66:15)
    at sharedProposalMiddleware (/app/node_modules/@connext/apps/dist/index.js:432:12)
    at proposeMiddleware (/app/node_modules/@connext/apps/dist/index.js:491:5)
    at validationMiddleware (/app/node_modules/@connext/apps/dist/index.js:438:23)
    at /app/node_modules/@connext/client/dist/channelProvider.js:60:15
    at Generator.next (<anonymous>)
    at /app/node_modules/@connext/client/dist/channelProvider.js:8:71
    at new Promise (<anonymous>)
    at __awaiter (/app/node_modules/@connext/client/dist/channelProvider.js:4:12)
    at validationMiddleware (/app/node_modules/@connext/client/dist/channelProvider.js:59:67)
    at /app/node_modules/@connext/cf-core/dist/protocol/propose.js:55:23
    at Generator.next (<anonymous>)
    at resume (/app/node_modules/@connext/cf-core/dist/protocol/propose.js:8:44)
    at /app/node_modules/@connext/cf-core/dist/protocol/propose.js:7:121
    at new Promise (<anonymous>)
    at Object.i.<computed> [as next] (/app/node_modules/@connext/cf-core/dist/protocol/propose.js:7:63)
    at ProtocolRunner.runProtocol (/app/node_modules/@connext/cf-core/dist/machine/protocol-runner.js:84:39)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async ProtocolRunner.initiateProtocol (/app/node_modules/@connext/cf-core/dist/machine/protocol-runner.js:44:33)
    Error: Error: Error: Cannot install an app with zero valued deposits for both initiator and responder.
rsercano commented 4 years ago

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 Service.ts and adding amount check to connext.

raladev commented 4 years ago

checked once more time, same result, but im sure that it was fine some time ago Screenshot from 2020-11-05 16-24-15

kilrau commented 4 years ago

@rsercano Roman stated:

if i remember correctly it was fine before https://github.com/ExchangeUnion/xud/pull/1942/commits/029eae80909fba570f4e5e0d1547285951ed42ca (at least there was no 500 err)

sangaman commented 4 years ago

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 Number casting and that's what was causing the issue, but I'm not sure how or where that got lost in the commits. Anyway I think once the tests pass this is good to squash merge.