fix(swapclients): check capacity up front #901
Conversation
This fixes a bug where the capacity for swap clients is thought to be 0 upon initialization. The checks to update the capacity happen on an interval but not right away. This adds logic to check the capacity immediately. Fixes #900.
ghost
assigned 
| @@ -321,6 +311,7 @@ class RaidenClient extends BaseClient { | |||
| * Returns the total balance available across all channels. | |||
| */ | |||
| public channelBalance = async (): Promise<ChannelBalance> => { | |||
| // TODO: refine logic to determine balance per token rather than all combined | |||
There was a problem hiding this comment.
Yeah just thought I'd make note of it. It's OK for now but eventually we'd want to know the breakdown.
| INSUFFICIENT_OUTBOUND_BALANCE: (currency: string, amount: number) => ({ | ||
| message: `${currency} does not have sufficient outbound balance of: ${amount}`, | ||
| INSUFFICIENT_OUTBOUND_BALANCE: (currency: string, amount: number, availableAmount: number) => ({ | ||
| message: `${currency} outbound balance of ${availableAmount} is not sufficient for order amount of ${amount}`, |
There was a problem hiding this comment.
Really like the more detailed error message :)
| @@ -50,7 +51,6 @@ func (cfg nodeConfig) genArgs() []string { | |||
| var args []string | |||
|
|
|||
| args = append(args, "--initdb=false") | |||
| args = append(args, "--nosanitychecks=true") | |||
There was a problem hiding this comment.
It's great that we get to test sanity checks and swaps now with simulation tests.
sangaman
force-pushed
the
fix/capacity-checks
branch
from
912a2d5 to
41833e3
Compare
This moves the logic to enable or clear the reconnection timer for swap clients to the `setStatus` method. This ensures we enable the timer any time we are disconnected and disable the timer when our connection is verified.
sangaman
force-pushed
the
fix/capacity-checks
branch
from
41833e3 to
87f257b
Compare
| if (!this.reconnectionTimer) { | ||
| this.reconnectionTimer = setTimeout(this.verifyConnection, BaseClient.RECONNECT_TIMER); | ||
| } else { | ||
| this.reconnectionTimer.refresh(); |
There was a problem hiding this comment.
If we decide to use the refresh API we'll also have to update the:
mininum required nodejs version to >=v10.2.0 and make sure we also update our simnet etc. machines. Using an older version of nodejs will crash my xud process. Otherwise the fix is working fine.
There was a problem hiding this comment.
Alternative solution (uglier, but less work) would be to just clear the timeout before setting a new one.
There was a problem hiding this comment.
You're right although I'm OK with going to node 10 as the minimum version, given that 8 is only going to be maintained through the end of the year. I'll open another PR to update the minimum version in package.json.
There was a problem hiding this comment.
I actually just went ahead and put it in this PR.
This raises the minimum Node.js version to support the `timeout.refresh` function.
Fixes #900.
This fixes a bug where the capacity for swap clients is thought to be 0 upon initialization. The checks to update the capacity happen on an interval but not right away. This adds logic to check the capacity immediately.
This disables the
nosanitychecksflag for the simulation tests, which will catch this bug should it regress.I also refactored some of the logic for setting and clearing the reconnection timer since some of that code overlapped with the fix.