Light-client: Integration test for send transfers

[christianbrb]

Description

As a follow up for #348 we need to test a send transfer. To keep the number of tests low, we should test a transfer with the highest complexity (mediated, fees, fees for PFS).

Acceptance criteria

• 10 Mediated transfers from the LC through a Raiden Python to a Raiden Python client
• Including mediation fees
• Including PFS fees
• Sequentially

Tasks

• [ ]

[nephix]

Blocked by https://github.com/raiden-network/light-client/issues/1307

[christianbrb]

One thought: As we have got receiving merged now, we could test the transfer like this
Light Client -> Python Client -> Light Client

If #1290 is merged we could also use webrtc?

[nephix]

Yes that should work

If #1290 is merged we could also use webrtc?

Yup I think so, only need to check whether the matrix server in the integration test image supports STUN etc

[andrevmatos]

We DO handle mediation fees properly in the light-client, for sending, receiving and mediating.
We just don't support setting (and deducting) fees different than zero from mediated transfers on #1315 , but I'll implement it before pushing for review.
Anyway, current master can be used without problems as transfers ends, one just needs to disable config.caps[Capabilities.NO_RECEIVE] to enable receiving.

[nephix]

We just don't support setting (and deducting) fees different than zero from mediated transfers

That's exactly what I meant

[kelsos]

Yes that should work

If #1290 is merged we could also use webrtc?

Yup I think so, only need to check whether the matrix server in the integration test image supports STUN etc

We need to create a new issue about enabling STUN on the integration image

[nephix]

We already have a hardcoded fallback google STUN server. If the container doesn't restrict outbound connections it probably works just fine

[kelsos]

It shouldn't but for the tests I would still like not to rely on anything external as it can introduce uncessary flakiness.

[andrevmatos]

The recommended matrix TURN server is coturn, and it provide full TURN capabilities, not only STUN (TURN is a superset of STUN which also supports relaying). Here is official guide for deployment, and when available, is always preferred over the fallback one.
The fallback also isn't hardcoded, it just defaults to Google's public server, but is exposed through the fallbackIceServers config option, therefore changeable through create's config param or Raiden.updateConfig method, if one needs to change it to something else for the tests.

[nephix]

The fallback also isn't hardcoded, it just defaults to Google's public server, but is exposed through the fallbackIceServers config option, therefore changeable through create's config param or Raiden.updateConfig method, if one needs to change it to something else for the tests.

https://github.com/raiden-network/light-client/pull/1290/files#diff-e0bab58c78de71147763f670fcc88688R107

Making something configurable doesn't magically make it not hardcoded

[andrevmatos]

Yes, it does, at least in my definition of hardcoded, which interprets the "hard" as "not changeable" without meddling with source code. That line you linked is in the function makeDefaultConfig, which creates the... default config, but is overwritable with the methods mentioned without needing to touch SDK's sources. Otherwise, every single param we have in the SDK would be hardcoded as well, which would make for a pretty shitty API, IMHO.

[nephix]

I know, yet it's hardcoded :)

[nephix]

I did a quick test and a direct transfer worked with both nodes. We have the following nodes:

• 0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec
• 0xCBC49ec22c93DB69c78348C90cd03A323267db86

When attempting a mediated tansfer

us
=> 0x517aAD51D0e9BbeF3c64803F86b3B9136641D9ec
=> 0xCBC49ec22c93DB69c78348C90cd03A323267db86

I'm getting an error from the PFS:

action {
type: 'path/find/failure',
payload: RaidenError: The requested target is offline.
at MergeMapSubscriber.project (/Users/nephix/Projects/light-client/raiden-ts/src/path/epics.ts:6201:15)
at MergeMapSubscriber._tryNext (/Users/nephix/Projects/light-client/raiden-ts/node_modules/rxjs/src/internal/operators/mergeMap.ts:135:21)
at MergeMapSubscriber._next (/Users/nephix/Projects/light-client/raiden-ts/node_modules/rxjs/src/internal/operators/mergeMap.ts:125:12)
at MergeMapSubscriber.Subscriber.next (/Users/nephix/Projects/light-client/raiden-ts/node_modules/rxjs/src/internal/Subscriber.ts:99:12)
at ThrowIfEmptySubscriber._next (/Users/nephix/Projects/light-client/raiden-ts/node_modules/rxjs/src/internal/operators/throwIfEmpty.ts:61:22)
at ThrowIfEmptySubscriber.Subscriber.next (/Users/nephix/Projects/light-client/raiden-ts/node_modules/rxjs/src/internal/Subscriber.ts:99:12)
at TakeSubscriber._next (/Users/nephix/Projects/light-client/raiden-ts/node_modules/rxjs/src/internal/operators/take.ts:92:24)
at TakeSubscriber.Subscriber.next (/Users/nephix/Projects/light-client/raiden-ts/node_modules/rxjs/src/internal/Subscriber.ts:99:12)
at ReplaySubject._subscribe (/Users/nephix/Projects/light-client/raiden-ts/node_modules/rxjs/src/internal/ReplaySubject.ts:80:20)
at ReplaySubject.Observable._trySubscribe (/Users/nephix/Projects/light-client/raiden-ts/node_modules/rxjs/src/internal/Observable.ts:238:19) {
details: { target: '0xCBC49ec22c93DB69c78348C90cd03A323267db86' },
name: 'RaidenError',
_code: undefined
},
meta: {
tokenNetwork: '0xE6B5eB09e1C8C50bA37B969Ced50D19B1A86C412',
target: '0xCBC49ec22c93DB69c78348C90cd03A323267db86',
value: BigNumber { _hex: '0x01' }
},
error: true
}

I remember having the same issue when the PFS didn't pick up the presence updates from Matrix correctly

[kelsos]

@nephix we should probably enhance the script to pull the service/node logs from the container before shutting it down at least with some debug flag going on

[kelsos]

do you have a draft somewhere?

[nephix]

@kelsos https://github.com/raiden-network/light-client/pull/1335

[kelsos]

With the merge of #1341 PFS should work properly outside the container.

From the debugging done on the existing test, the steps needed before the transfer are the following:

• getAvailability of the target (otherwise we won't have any presence information since the target is not monitored)
• mint some SVT service tokens and deposit to the user deposit contract. The SVT_TOKEN_ADDRESS environment variable should be available with the address of the SVT token in the integration image.

[andrevmatos]

My 2cents, as a side-node: no need to cache getAvailability, it's cached internally, so one can just call it directly every time. We may look at some point into having it called from transfer and/or findRoutes, but for now, it's kind of expected it'd be called before starting the transfer. Availability is already fetched for partners, so there would be no need to call it on direct transfers, but doesn't hurt either, so better have it as part of the standard workflow