The getConnectors method on the ledger plugin should be viewed as an automatic configuration (similar to getPrefix) and connectors should be able to override it via their configuration.

We should assume that most connectors will pass in a list of connectors via the config and using getConnectors should be the exception (although still supported.)

Proposed config

# Provide a basic list of peers
CONNECTOR_PEERS=https://connie.ilpdemo.org,https://fred.example.com # default = ''

# Whether to automatically populate the list of peers by calling getConnectors
# on all ledger plugins
CONNECTOR_AUTOLOAD_PEERS=true # default = false

As discussed in interledger-deprecated/ilp-kit#59, the connector should not cause problems even if one or more of its ledgers is offline when it starts up. The connector should keep trying to connect to the ledger that's down (or hasn't started), and only offer to facilitate payments once it has successfully connected. It should also stop facilitating payments if it is temporarily disconnected from a ledger.

Both the precision and balance cache assume they should talk to a five-bells-ledger, they should instead use methods on the multiledger abstraction

Add quote error for not enough liquidity in market maker account

Multiledger does not use a store field when instantiating LedgerPlugins, even though in ILP RFC 0004 a store is a required field for PluginOptions

With interledger-deprecated/five-bells-ledger/pull/104, we must pass authentication to the ledger's subscription endpoints. five-bells-connector/lib/ledgers/five-bells-ledger.js must include authentication in the subscribe and unsubscribe methods.

• Running tests
• Branching
• Tagging releases
• Release process

While a db is not needed for #77, it is needed to ensure that Universal mode connectors do not lose money if the server crashes while waiting for some destination transfers to be executed. The services/destinationSubscriptions.js and services/sourceSubscriptions.js should use a db rather than an in-memory map.

Connectors should store the details of payments they've facilitated and allow clients to query them

@emschwartz
Hi Evan,
I would like to connect 2 private ledgers. Interledger seems very suitable for that. I've run five-bells-demo and looked into the code. However, I still have misunderstanding. You use js-ilp-plugin-bells and five-bells-connector. Unfortunately, I didn't find unified connector implementation (only for five-bells demo). To achieve my plans I need my own plugin and unified connector as well, isn't it?

Regarding plugin: I know you already have some different plugins, cool. What kind of plugins do you plan to implement and when?

Prerequisite for #269. Please see that ticket for more context / motivation.

This should be implemented in a backwards-compatible way, meaning if a connector does not receive scale and precision information from another connector it should behave as it does now. It MUST always broadcast scale and precision for any ledger that it is the final destination for. For any other destination, it will forward the scale and precision information (or lack thereof) that it received.

If connectors receive 500 or other errors from the ledgers while putting transfers they should retry sending them. This is extremely important on the execution side in Universal mode, because if they don't retry they will lose money when transfers time out

Right now the connector swallows the plugin debug statements

The connector should have a default key and the ability to pass in a specific key per ledger via CONNECTOR_CREDENTIALS. If an account is set up to use the default key, that ledger does not need to appear in CONNECTOR_CREDENTIALS at all.

See:

The connector should know its minimum_allowed_balance and calculate its available balance before comparing it to the amount, instead of just using the nominal balance.

route-broadcaster.js makes a calls the getAccount method of a LedgerPlugin on line 126, although the method is not a part of the LedgerPlugin interface.

The connector should be able to interface with multiple different kinds of ledgers. The connector has the multiledger abstraction, but there are a number of places (e.g. balanceCache, testPaymentExpiry) where other parts of the code assume the connector is dealing with only five-bells-ledgers. All of the functionality necessary for interacting with the ledgers should be moved into the multiledger abstraction and all other parts of the code should use that abstraction layer.

The config will also need to be changed so that the ledger-related options specify which type of ledger they are for.

Even though source_account and destination_account are not currently needed, it would be nice to be able to provide them, such that the /quote endpoint can generate a complete, ready-to-use payment object.

See five-bells-shared for examples.

Currently connectors are aware of each other's HTTP URIs. In practice, they should instead use ledger plugins to communicate, so that the communication mechanism is left up to the specific ledger and is not part of ILP.

See interledger/rfcs#96

Since ILP is intended to be able to support more than just currency exchange, we should rename the "thing" in the connector currently called "currency" to be "asset".

Looking at the code, It appears that the connector endpoint is already called "/pairs" and uses the term "asset" in its property values, so perhaps this is just a documentation change for this page?

In "optimistic" mode, the sender sends a payment with no condition. It is completely up to the connectors to pass on the payment. Honest connectors should.

This used to work in the connector but now it returns a UnacceptableExpiryError: Destination transfers with execution conditions must have an expires_at field for connector to agree to authorize them. This is because the validateExpiry function assumes that if the payment is a Universal mode payment if it is not an Atomic mode payment. Here it checks the expiry, but it should not be a problem if there is no expiry set and there is no condition set.

If the connector gets a quote request for an amount that is out of range for one of its ledgers it will round the amount, in certain cases, down to 0 and then return a generic UnprocessableEntity error. It should return an error that more specifically describes the problem

In the error case reported in RILP-254, Connector sets 'authorized' flag in debit to 'true', even though the transfer object is in 'proposed' state (i.e., not yet in 'prepared' state).
It is set in settlt() function in src/lib/payments.js.
I have talked with @justmoon and he thinks Connector should not do this.

connector-payment.json.txt
connector-receiver-transfer.json.txt
connector-sender-transfer.json.txt

It should be possible to use Postgres as the backend for the connector store.

This would be used by ILP Kit for instance.

When you have a single bells plugin and no pairs, the connector prints an error on startup.

Steps to reproduce

• Set up a connector with a single ilp-plugin-bells, e.g.

  CONNECTOR_LEDGERS={"ilpdemo.red.":{"currency":"USD","plugin":"ilp-plugin-bells","options":{"account":"https://red.ilpdemo.org/ledger/accounts/alice","password":"alice"}}}
  

• Make sure no pairs are configured

Expected behavior

The connector starts up and simply doesn't trade (since it has no pairs.)

It prints a warning that it has been configured with zero pairs and won't do any trading.

Actual behavior

The connector throws a cryptic exception:

[connector] 2016-11-30T18:30:15.255Z connector:app error Error: Prefix has not been set
[connector]     at FiveBellsLedger.getPrefix (/opt/ilp-kit/node_modules/ilp-plugin-bells/src/lib/plugin.js:325:29)
[connector]     at RouteBroadcaster._crawlClient (/opt/ilp-kit/node_modules/ilp-connector/src/lib/route-broadcaster.js:100:45)
[connector]     at next (native)
[connector]     at onFulfilled (/opt/ilp-kit/node_modules/co/index.js:65:19)
[connector]     at /opt/ilp-kit/node_modules/co/index.js:54:5
[connector]     at Promise._execute (/opt/ilp-kit/node_modules/bluebird/js/release/debuggability.js:299:9)
[connector]     at Promise._resolveFromExecutor (/opt/ilp-kit/node_modules/bluebird/js/release/promise.js:481:18)
[connector]     at new Promise (/opt/ilp-kit/node_modules/bluebird/js/release/promise.js:77:14)
[connector]     at co (/opt/ilp-kit/node_modules/co/index.js:50:10)
[connector]     at toPromise (/opt/ilp-kit/node_modules/co/index.js:118:63)
[connector]     at Array.map (native)
[connector]     at arrayToPromise (/opt/ilp-kit/node_modules/co/index.js:154:26)
[connector]     at toPromise (/opt/ilp-kit/node_modules/co/index.js:120:49)
[connector]     at next (/opt/ilp-kit/node_modules/co/index.js:99:29)
[connector]     at onFulfilled (/opt/ilp-kit/node_modules/co/index.js:69:7)
[connector]     at /opt/ilp-kit/node_modules/co/index.js:54:5
[connector] npm run start-prod-connector exited with code 0

What's happening?

@sharafian and I already did some debugging and what is happening here is that the pairs initialization is what actually calls connect on the various plugins. If there are no pairs, the plugins don't get initialized, so when we reach RouteBroadcaster#start, the plugins aren't connected. Since RouteBroadcaster calls getPrefix, we get the above error.

Based on how our current iterations of the routing and quoting protocols, we should be able to let connector round destination amounts to the correct precision when quoting. There are several cases to handle:

1. Destination is local

The connector can get the precision from the local destination metadata.

It will also attach precision and scale information when broadcasting local routes.

2. Destination is not local but is included in routing table

In this case, the routing table will contain the correct precision and scale and therefore allows the connector to round correctly.

When rebroadcasting incoming routes, the connector will forward the precision and scale information.

3. Destination is not local and not included in routing table, but a prefix route exists

When quoting for a prefix route (a route that aggregates multiple destination ledgers via a single prefix), the connector will recursively call the next connector in the chain for a quote.

4. Destination is not local and not included in routing table and no prefix route exists

In this case, the connector will return an AssetsNotTradedError when quoting.

5. Destination uses a Tail Routing Table (TRT)

We don't yet support the Tail Routing Table (TRT) feature, but assume that this will be added in the future. The way rounding works with a TRT is that the TRT must contain the scale and precision information for the final destination ledger. However, it does not need to contain precision and scale for any other ledgers in the tail.

Most of the bottom of both src/backends/one-to-one.js and src/backends/fixerio/index.js are identical.

A simple GUI, in the same spirit as Futon for Couchdb

While updating the unit tests to work with five-bells-shared/pull/169, I found a rather odd bug related to tv4: When the function validate(schemaId, json) is called, json is successfully validated even if schemaId was not added before. The expected behavior would be that function validates throws an error along the lines unknown schemaId, but it does not.

Since the new validator introduced with five-bells-shared/pull/169 throws an error message as expected, this test case fails. The reason for the failure is that the schema Quote.json is not added. Quote.json is in ilp-connector/schemas, but services/validate.js loads the schemas from five-bells-shared/schemas and, thus, does not load Quote.json.

Simply copying the schemas from ilp-connector/schemas to five-bells-shared/schemas does not solve it.

On a side note: Does the current connector implementation validates quote request based on the schema? Does not look like, or I might have missed the code.

Add the ability to supply database configuration for persisting payments.

config.js will throw a fatal error if 'username' and 'password' fields are not present in the CONNECTOR_CREDENTIALS, even though not all ledger plugins (such as ILP-Plugin-Virtual) don't need a username/password.

If a user provides source_account and/or destination_account in a /quote request, the connector should automatically determine the corresponding {source|destination}_ledger value.

Ideally without string manipulating any URIs. ;)

Instead of loading the backend from a local folder, the connector should treat backends the same way it treats ledger plugins and require the name that's given so you can more easily use a custom one.

We should also document the API expected from a backend, and document the environment variables for setting it up.

Update any README or source code references to "five-bells-trader". Should be connector as in the white paper and other ILP write-ups.

Replace multiledger.js with ilp-core.

When the connector checks the balance in its account on a ledger (here) it should take the minimum_allowed_balance field in case it is allowed to go negative

On the current connector, quote requests like GET /quote/USD/EUR/-1.0/source?scale=2 do not report an error. The connector should probably only accept strictly positive amounts.

This field is not in the LedgerPlugin interface, but ledger.id field is used later in the subscription service.

SonarQube shows 28 errors in the ilp connector code

Most of these (25) are places where there should be a yield" statement added to the generator

Example code missing a yield in src/backends/fixerio/index.js, 63
/**

• Get backend status
  */
• getStatus () {
  return {
  backendStatus: healthStatus.statusOk
  }
  }
  "A generator without a yield statement is at best confusing, and at worst a bug in your code, since the iterator produced by your code will always be empty."

Here are the other places where this occurs:
src/backends/fixerio/index.js
Line 63
Line 93
Line 143

src/backends/ilp-quoter/index.js
Line 66

src/backends/one-to-one.js
Line 28
Line 34
Line 48
Line 91

src/errors/assets-not-traded-error.js
Line 8

src/errors/external-error.js
Line 7

src/errors/invalid-amount-specified-error.js
Line 7

src/errors/no-amount-specified-error.js
Line 7

src/errors/no-related-destination-debit-error.js
Line 8

src/errors/no-related-source-credit-error.js
Line 8

src/errors/unacceptable-amount-error.js
Line 7

src/errors/unacceptable-conditions-error.js
Line 8

src/errors/unacceptable-expiry-error.js
Line 8

src/errors/unacceptable-quoter-amount-error.js
Line 7

src/errors/unacceptable-rate-error.js
Line 8

src/errors/unrelated-notification-error.js
Line 8

src/errors/unsupported-pair-error.js
Line 8

src/models/quote.js
Line 6
Line 24

src/models/subscriptions.js
Line 8

src/lib/message-router.js
Line 108

In addition, there are a few other issues:

src/lib/config.js
Remove this useless assignment to local variable "ledgerCredentials"
Line 206

src/lib/message-router.js
Change this condition so that it does not always evaluate to "true".
Line 162

src/backends/ilp-quoter/index.js
Change this condition so that it does not always evaluate to "true".
Line 60

This issue is a follow up to #94

To fix RILP-254,
(1) Connector should not trust any transfer (e.g. the ones from Sender). It should only trust the notification from Ledger.
(2) Connector should verify the notification from Ledger, e.g., via digital signature.

Issue 94 fixes (1).
This new issue is for (2).

interledger-deprecated/five-bells-ledger#133 removed the execution_condition_fulfillment from the notification. It either needs to be added back, or the notification will have to send the fulfillment instead of the transfer. The latter solution requires changes to the connector.

Remove this code:

https://github.com/interledger/five-bells-connector/blob/f223d7df1f2cd3820ad318832d5dff3bc6189eb1/src/models/subscriptions.js#L12

https://github.com/interledger/five-bells-connector/blob/f223d7df1f2cd3820ad318832d5dff3bc6189eb1/src/models/subscriptions.js#L21

and make it the responsibility of the plugins to assign the ledger property.

Here's the feedback I got

I've set CONNECTOR_FX_SPREAD=0 CONNECTOR_SLIPPAGE=0.

When I transfer £10 from wallet a to wallet b, £10 is sent from a but only £9.99 is received by wallet b. Is there another parameter to change that could account for the £0.01 lost or is this a rounding error?

If you request a quote from a connector from one account to another on the same ledger, right now the connector won't recognize that there is a direct path and it will give you a path from ledger A -> B -> A instead. The connector should (take pity on you and) return a simple transfer object with no nested transfers (and probably no fee)

As far as I can tell, all connectors effectively need to have the same route broadcast interval and expiration time. If a connector has rates that don't change very often, they might not want to waste the effort rebroadcasting the same data over and over again. However, right now the other connectors would expire the routes. This may even create an incentive to have a lower than average time in case there are some connectors with lower times.

Routes should probably carry something like cache control headers so you know how long another connectors' routes are valid for, rather than expiring everything on your own interval.