interledger-deprecated / ilp-plugin-payment-channel-framework Goto Github PK

Calling send_request with:

[
  {
    "ledger": "g.crypto.paychan.framework.",
    "to": "g.crypto.paychan.framework.server",
    "ilp": "ARwAAAAAB1TUwA5nLnVzLm5leHVzLmJvYgMEEEEA"
  }
]

Result:

{
    "id": "InvalidFieldsError",
    "message": "must have a destination (.to)"
}

To fix this error, from: ... has to be added to the request, which does not make much sense given the error message and the request has already a to field.

When looking at a debug track, it becomes apparent what goes wrong:

[api]   ilp-plugin-virtual incoming send_request from [ 'https://wallet2.example/api/pseers/rpc' ] with [ { ledger: 'g.crypto.paychan.framework.',
[api]     to: 'g.crypto.paychan.framework.server',
[api]     ilp: '...' } ] +0ms
[api] validate incoming
[api] ASDF { ledger: 'g.crypto.paychan.framework.',
[api]   to: 'g.crypto.paychan.framework.server',
[api]   ilp: '...' }
[api] validate outgoing
[api] ASDF { ledger: 'g.crypto.paychan.framework.',
[api]   to: undefined,
[api]   from: 'g.crypto.paychan.framework.server',
[api]   ilp: '...' }

The from field from the incoming message is used to set the to field in the outgoing message. Since from is undefined in the incoming message, to becomes undefined in the outoing message.

We might want to remove .to and .from from the message to avoid such bugs. In a bilateral trustline, these field are redundant.

Related to interledger/rfcs/issues/234 (cc: @sharafian @michielbdejong)

Not yet implemented it seems, see https://github.com/interledgerjs/ilp-plugin-payment-channel-framework/blob/mj-more-robust-listener/src/lib/plugin.js#L160

We should be careful not to repeat XRPLF/xrpl.js#792 - maybe it would make sense to create a tiny 'websocket client' module that correctly reconnects and disconnects, and use that across ripplelib, plugin-bells, plugin-framework, and btp-toolkit?

The auth token should not be passed to the server in the URL like this: https://github.com/interledgerjs/ilp-plugin-payment-channel-framework/blob/bs-clp/src/model/rpc.js#L178-L183

Instead, send the auth token as the first message once the WebSocket connection is open.

It occurred to me that in the WebSocket server case, we're doing this wrong. :)

Currently, we're telling one plugin to listen on a port. That will work for a sender or a receiver that just has one peer (e.g. my ILSP will connect to my online shop server's WebSocket server to deliver BTP Prepare messages, but I don't expect anyone else to ever connect to that WebSocket URL).

What would be more correct would be to have one Plugin per WebSocket connection. So:

const opts = {
 listenOnPort: 443,
 cert: <bla.pdx>
}

const factory = new PluginFactory(opts)

factory.on('clientConnected', async (plugin) => {
  // now we have one plugin, because one peer connected to the WebSocket server
  // which the factory is running.
  await plugin.connect()
  await plugin.sendTransfer(foo)
  await plugin.disconnect()
})

The fulfillment data should be binary in:

• https://github.com/interledgerjs/btp-packet/blob/master/index.js#L131
• https://github.com/interledgerjs/btp-packet/blob/master/index.js#L191
• https://github.com/interledgerjs/btp-packet/blob/master/index.js#L225
• https://github.com/interledgerjs/btp-packet/blob/master/index.js#L407
• https://github.com/interledgerjs/btp-packet/blob/master/index.js#L399
• https://github.com/interledgerjs/ilp-plugin-payment-channel-framework/blob/master/src/model/rpc.js#L325
• https://github.com/interledgerjs/ilp-plugin-payment-channel-framework/blob/master/src/lib/plugin.js#L476

And so it should be base64-decoded in https://github.com/interledgerjs/ilp-plugin-payment-channel-framework/blob/master/src/lib/plugin.js#L474

plugin.js:437 instantiates the error class AlreadyRejectedError, but this error class does not exist.

From looking into the LPI docs, I assume this should be an AlreadyRolledBackError error.

PluginVirtual basically has two functions: the ledger and the codec.

While working on https://github.com/interledgerjs/amundsen/pull/2/files#diff-034b86edf6fcd40d95d1588b485d6d9e it occurred to me that it would probably make sense to make the virtual ledger inside src/lib/plugin.js more explicit.

If we define an in-memory ledger, whose interface is the LPI, it can:

• expose the LPI interface of the plugin (e.g. to the connector when this plugin is used in ilp-connector)
• keep track of the balance
• expose an LPI interface to the outside (e.g. to the peer when used to connect to a peer).
• ledger.myPlugin.getBalance() === -ledger.peerPlugin.getBalance()
• ledger.peerPlugin.sendTransfer() triggers ledger.myPlugin.on('incoming-prepare')
• etc.

A separate part of the code can then act as a codec, deal with translating between LPI and BTP and back (see interledger-deprecated/btp-toolbox#2 for an example).

Separating the peer-ledger, which exposes two LPI interfaces (ledger.myPlugin and ledger.peerPlugin) makes LPI more important and confines the impact of BTP to inside the codec. A nice separation of concerns, maybe?

The repo URL is "url": "git+https://github.com/interledger/ilp-plugin-virtual.git", but should be "url": "git+https://github.com/interledger/ilp-plugin-payment-channel-framework.git"

Sorry I'm new to Interledger, I already setup ILP-KIT (3 instances) my own, now I try to make transfer between kits.

My question is:

"server": "btp+wss://username:[email protected]:1234/example_path"

What exactly username in config bellow come from? it's an account in ILP-KIT, or connector account in five-bell-ledger?

This line uses the JSON stringified version of the additional_info as the ILP error data. That makes no sense, it should pass through the rejectionReason.message. Similarly on this line it puts the message inside a JSON object for some reason.

More details here: #17 (comment)

Trying to fulfill a non-existant transfer gives a misleading error message. Example:

POST /rpc/?method=fulfill_condition&prefix=peer.me HTTP/1.1
...
[
  "0e798bd6-213b-4b2f-bc1c-040788e7bae5" // does not exist
]

Error:

{
    "id": "TypeError",
    "message": "Cannot read property 'expiresAt' of undefined"
}

The config files required to run plugin-xrp-paychan's integration tests are hardcoded in circle.yaml, which is a little janky.

Putting the config files in the integration test repo is not a solution. The config files need to be loaded (and rippled has to be started) before the integration tests are installed.

Method get_limit is only added if the plugin is stateful and has a payment channel, see: https://github.com/interledgerjs/ilp-plugin-payment-channel-framework/blob/master/src/lib/plugin.js#L98

get_limit should also be added if the plugin is stateful, but does not have a payment channel.

If multiple MaxValueTrackers are instantiated for the same key, the tracker will not work as expected. I added some test cases to demonstrate the problem. Here is the result of running the test caess.

cc @sharafian

Moved from interledgerjs/ilp-plugin-xrp-paychan#15.

For plugin-virtual it makes sense to talk about maxBalance, but I think in the payment channel framework, maxUnsecured is a better name?

Username and password provided in the connection string are not decoded. For example, providing a client plugin with the option server: btp+ws://username with blanks:shh its a [email protected]:3333 will result in user and pass being parsed as username%20with%20blank and shh%20its%20a%secret, respectively.

This test case should not fail.

When an error occurs in the payment channel handler for handleIncomingPrepare, the plugin ceases to accept incoming RPC connections, and also disconnects all of its sockets. I'm not certain why this occurs, but I'm looking into it.