Prevent Raiden from being first swap leg #1072
Conversation
|
Nice! Tests need to be fixed. |
| @@ -5,6 +5,7 @@ class HttpService { | |||
| constructor(private service: Service) {} | |||
|
|
|||
| public resolveHashRaiden = async (resolveRequest: RaidenResolveRequest): Promise<RaidenResolveResponse> => { | |||
| // TODO: add reveal_timeout, settle time out, token, etc | |||
There was a problem hiding this comment.
Mainly curious, what will we need these for?
There was a problem hiding this comment.
we need to verify that we got the expected reveal_timeout (as we agreed during the swap preparation (if we are not preventing raiden to be the first leg))
We also need to check that we got the payment with the correct token.
| @@ -433,6 +433,19 @@ class Swaps extends EventEmitter { | |||
|
|
|||
| const { makerCurrency, makerAmount, takerCurrency, takerAmount } = Swaps.calculateMakerTakerAmounts(quantity, price, isBuy, requestBody.pairId); | |||
|
|
|||
| const makerSwapClient = this.swapClientManager.get(makerCurrency)!; | |||
There was a problem hiding this comment.
Looks like tests are failing because of this line, we assert that maker swap client is not null but it's not necessarily the case. Previously we were calling isPairSupported before getting any swap clients, all that method does is check that we have connected swap clients for both sides of the pair. You can move that call to before this line (and use requestBody.pairId) and it should fix the test failing.
lib/swaps/Swaps.ts
Outdated
| switch (deal.role) { | ||
| case SwapRole.Maker: | ||
| expectedAmount = deal.makerAmount; | ||
| source = 'Taker'; | ||
| destination = 'Maker'; | ||
| if (deal.makerCltvDelta! > 50 * 2) { | ||
| this.failDeal(deal, SwapFailureReason.UnknownError, 'Wrong CLTV received on first leg'); |
There was a problem hiding this comment.
Maybe use SwapFailureReason.InvalidResolveRequest instead of the unknown error?
|
@sangaman I created this PR as a starting point for the change. Please take ownership and make changes as needed. |
This prevents Raiden based currencies from being the first leg of a swap. It better calculates the maker CLTV by using a cltv-delta of 5760 for raiden. This is the first step towards a more robust approach. Co-authored-by: Daniel McNally <[email protected]>
sangaman
force-pushed
the
raiden-first-leg
branch
from
f92da12 to
7c76a3d
Compare
|
Gotcha, I updated the PR with some minor changes per my comments to make the tests pass. |
| @@ -5,6 +5,7 @@ class HttpService { | |||
| constructor(private service: Service) {} | |||
|
|
|||
| public resolveHashRaiden = async (resolveRequest: RaidenResolveRequest): Promise<RaidenResolveResponse> => { | |||
| // TODO: add reveal_timeout, settle time out, token, etc | |||
This PR prevents Raiden from being first leg of swap.
It better calculates the maker CLTV by using cltv-delta of 5760 in the case of raiden.
This is only a basis for a better change