Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid quoteQuery checking #28

Closed
alkopnin opened this issue Aug 24, 2016 · 26 comments
Closed

Invalid quoteQuery checking #28

alkopnin opened this issue Aug 24, 2016 · 26 comments

Comments

@alkopnin
Copy link

alkopnin commented Aug 24, 2016

Hi guys,

I investigate how to integrate Interledger with two private ledgers.

Previously you said that I can use ilp directly with five-bells-connector. Seems you have an error with quoteQuery params checking.

Have a look here:

connector-name 2016-08-24T13:25:23.356Z koa INFO <-- GET /quote?source_address=http%3A%2F%2Flocalhost%3A3001%2Filp_account&destination_address=http%3A%2F%2Flocalhost%3A3001%2Filp_account&destination_amount=50
connector-name 2016-08-24T13:25:23.362Z error-handler WARN Invalid URI parameter: Missing required parameter: source_ledger or source_account

connector-name 2016-08-24T13:26:27.834Z koa INFO <-- GET /quote?source_ledger=http%3A%2F%2Flocalhost%3A3001%2Filp_account&destination_address=http%3A%2F%2Flocalhost%3A3001%2Filp_account&destination_amount=50
connector-name 2016-08-24T13:26:27.840Z error-handler WARN Invalid URI parameter: Missing required parameter: destination_ledger or destination_account
`

To solve it you can either make changes here ilp-core->src->lib->client.js or modify five-bells-connectors

For client.js you can do:
const quoteQuery = {
source_ledger: (yield plugin.getAccount()),
...
destination_ledger: params.destinationAddress,
...
}

@emschwartz
Copy link
Member

What versions of the client and connector are you running?

@alkopnin
Copy link
Author

ilp: "version": "4.0.2"
ilp_core: "version": "7.0.1"
five-bells-connector: "version": "7.0.3"

@emschwartz
Copy link
Member

Looking into it. I think there is a mismatch between functionality of some of those components. The integration tests weren't being run for js-ilp and js-ilp-core, which is one reason that wasn't spotted before, so I'm working on #29 and interledger-deprecated/ilp-core#44

@alkopnin
Copy link
Author

Okey, cool. Thanks. Is it means that I can face some major bugs, or mostly it should be stable?

@emschwartz
Copy link
Member

It's possible the integration between js-ilp and the other components isn't so solid cause that repo's newer, and we haven't had the integration tests enabled, but barring that it should be working. Sorry you're running into trouble

@alkopnin
Copy link
Author

alkopnin commented Aug 24, 2016

no worries
Evan, I also have a trouble with getting a quote: "Assets Not Traded: This connector does not support the given asset pair". I bit confused how to use pairs, I set up them into the Connectors, but how I can trade them?

I have 2 ledgers and 2 connectors as well:
1 Connector pairs:
[{"source_asset":"FAB1","source_ledger":"http://localhost:3001","destination_asset":"FAB2","destination_ledger":"http://localhost:3002"},{"source_asset":"FAB2","source_ledger":"http://localhost:3002","destination_asset":"FAB1","destination_ledger":"http://localhost:3001"}]

2 Connector pairs:
[{"source_asset":"FAB2","source_ledger":"http://localhost:3002","destination_asset":"FAB1","destination_ledger":"http://localhost:3001"},{"source_asset":"FAB1","source_ledger":"http://localhost:3001","destination_asset":"FAB2","destination_ledger":"http://localhost:3002"}]

Issue happens when I ask a quote, in my opinion, it should work. Also, could you clarify me, please, what is the quote exactly?

UPD: Output
request: { packet: {
amount: '50',
account: 'http://localhost:3001/node1_ledger',
data: { expires_at: '2016-08-24T15:45:58.162Z', request_id: '950ca3cd-0495-4783-b202-e96cd0955bab' } },
condition: 'cc:0:3:Ssdu754gYMIGYonbkGcEOsHHlfJYcEO5fpR5OxJbfws:32' }

2016-08-24T15:45:28.228Z koa INFO <-- GET /quote?source_ledger=http%3A%2F%2Flocalhost%3A3002%2Fnode2_ledger&destination_ledger=http%3A%2F%2Flocalhost%3A3001%2Fnode1_ledger&destination_amount=50
2016-08-24T15:45:28.233Z error-handler WARN Assets Not Traded: This connector does not support the given asset pair
2016-08-24T15:45:28.237Z koa INFO --> GET /quote?source_ledger=http%3A%2F%2Flocalhost%3A3002%2Fnode2_ledger&destination_ledger=http%3A%2F%2Flocalhost%3A3001%2Fnode1_ledger&destination_amount=50 422 9ms -

@emschwartz
Copy link
Member

The pairs thing is admittedly a bit confusing. Try using CONNECTOR_LEDGERS='["FAB1@3001.","FAB2@3002."]' and CONNECTOR_CREDENTIALS where the keys in that object are 3001. and 3002. (don't forget the period afterwards either).

You can check that it worked by going to the connector's /pairs endpoint.

In general quotes tell you how much you need to pay a connector in order for them to pay out a certain amount (or vice versa). For js-ilp the result of the quoteRequest method is everything you need to pass to payRequest in order for it to fulfill the request.

@alkopnin
Copy link
Author

It seems my ledger plugin don't set into multiledger.js module.

Well, I add some trace and figure out:

  1. after asking quote with ilp-src-lib-sender.js-client.quote() I've got:
    $ f-b-connector-src-models-quote.js-getQuote
    $ RoutingTables { baseURI: 'http://localhost:4001', expiryDuration: 45000, sources: {}, accounts: {} }
    $ hop not found

  2. sources is empty. Routing-tables creates with empty list by default, but you have a method to set up sources after broadcast will start
    (sources is used for _f-b-connector-src-lib-route-builder.js-findNextHop)
    $ f-b-routing-src-lib-routing-tables-ctor []

  3. When broadcast starts I've got
    $ f-b-connector-src-lib-route-broadcaster-start
    $ 2016-08-25T13:58:52.393Z app ERROR Error: connect ECONNREFUSED 127.0.0.1:3001
    at Object.exports._errnoException (util.js:870:11)
    at exports._exceptionWithHostPort (util.js:893:20)
    at TCPConnectWrap.afterConnect as oncomplete

  4. I have look at route-broadcaste.js
    Before update routes (func reloadLocalRoutes) you call: yield this.crawLedger()
    Where this.ledgers.getLedgers() will invoke

  5. ledgers comes from outside (service/route-broadcaster.js) with multiledger

  6. and here (multiledger.js) your use /ledgers/five-bells-ledger to operate with ledgers instead of mine plugin
    $ not mine plugin
    $ connector-fabric f-b-connector-src-lib-route-broadcaster-_crawLedger Promise { }

I bit confused, could you help me, please. Either I do something wrong or Connector don't work correctly. Do I have to set special flags of Connector or maybe configure it especially?

Thanks advance.

@emschwartz
Copy link
Member

Please try what I suggested before and see if the pairs show up in the /pairs response

@emschwartz
Copy link
Member

I think your connector version is quite out of date actually. The latest is 9.2.1

@alkopnin
Copy link
Author

alkopnin commented Aug 25, 2016

Yes, sure.
What do you mean:
"CONNECTOR_CREDENTIALS where the keys in that object are 3001. and 3002."

Accordingly the sample my Credentials looks like:
{ account_uri: ledger + '/accounts/' + encodeURIComponent(name),
username: name,
password: name }

@emschwartz
Copy link
Member

@alkopnin
Copy link
Author

alkopnin commented Aug 25, 2016

Seems fail with five-bells-ledger.js as well

2016-08-25T16:01:40.518Z app INFO connector listening on 0.0.0.0:4002
2016-08-25T16:01:40.521Z app INFO public at http://localhost:4002
2016-08-25T16:01:40.522Z app INFO pair FAB1@3001.
2016-08-25T16:01:40.522Z app INFO pair FAB2@3002.
2016-08-25T16:01:40.523Z fixerio DEBUG connect
2016-08-25T16:01:41.041Z app ERROR Error: Invalid URI "3001./connectors"
at Request.init (/home/akopnin/work/test-interledger/node2/node_modules/request/request.js:275:31)
at new Request (/home/akopnin/work/test-interledger/node2/node_modules/request/request.js:129:8)
at request (/home/akopnin/work/test-interledger/node2/node_modules/request/index.js:55:10)
at /home/akopnin/work/test-interledger/node2/node_modules/co-request/index.js:21:23
at /home/akopnin/work/test-interledger/node2/node_modules/co-request/index.js:18:16
at FiveBellsLedger._getConnectors (/home/akopnin/work/test-interledger/node2/node_modules/five-bells-connector/src/ledgers/five-bells-ledger.js:212:23)
at next (native)
at onFulfilled (/home/akopnin/work/test-interledger/node2/node_modules/co/index.js:65:19)
at /home/akopnin/work/test-interledger/node2/node_modules/co/index.js:54:5
at FiveBellsLedger.co (/home/akopnin/work/test-interledger/node2/node_modules/co/index.js:50:10)
at FiveBellsLedger.createPromise (/home/akopnin/work/test-interledger/node2/node_modules/co/index.js:30:15)
at FiveBellsLedger.getConnectors (/home/akopnin/work/test-interledger/node2/node_modules/five-bells-connector/src/ledgers/five-bells-ledger.js:208:41)
at RouteBroadcaster._crawlLedger (/home/akopnin/work/test-interledger/node2/node_modules/five-bells-connector/src/lib/route-broadcaster.js:81:37)
at next (native)
at onFulfilled (/home/akopnin/work/test-interledger/node2/node_modules/co/index.js:65:19)
at /home/akopnin/work/test-interledger/node2/node_modules/co/index.js:54:5

@emschwartz
Copy link
Member

Which version of the connector is that with?

@alkopnin
Copy link
Author

At this moment this is five-bells-connector: "version": "7.0.3"

I've seen info about old version. I'm going to update connector to 9.2.1 and tell you results Today later.
Thank you

@emschwartz
Copy link
Member

That error makes sense then, because one of the big breaking changes introduced in the latest versions was the new ILP address format (that's the dot-separated ledger/account bit)

@alkopnin
Copy link
Author

alkopnin commented Aug 26, 2016

After update I still have an error. Seems my sources is empty and I can't get a hop.:

$ [2016-08-26T11:56:21.226Z] INFO: connector/11135 on akopnin-ubuntu: creating quote sourceAddress=http://localhost:3002/node1_ledger sourceAmount=undefined destinationAddress=http://localhost:3001/node2_ledger destinationAmount=50 destinationPrecisionAndScale=undefined slippage=undefined (module=route-builder)
$ ilpcore-src-lib-core.js-_quote getting hop
$ ilp-core-src-lib-core.js-_findBestHopForAmount http://localhost:3002/node1_ledger http://localhost:3001/node2_ledger undefined 50
$ ilp-core-src-lib-core.js-_findBestHopForAmount tables is RoutingTables {
$ baseURI: 'http://localhost:4002',
$ localTables:
$ RoutingTables {
$ baseURI: 'http://localhost:4002',
$ expiryDuration: 45000,
$ sources: PrefixMap { prefixes: [], items: {} },
$ accounts: {} },
$ publicTables:
$ RoutingTables {
$ baseURI: 'http://localhost:4002',
$ expiryDuration: 45000,
$ sources: PrefixMap { prefixes: [], items: {} },
$ accounts: {} } }
$ [2016-08-26T11:56:21.231Z] WARN: connector/11135 on akopnin-ubuntu: Assets Not Traded: This connector does not support the given asset pair (module=error-handler)

Pairs are
http://localhost:4002/pairs
[{"source_asset":"FAB2","source_ledger":"3002.","destination_asset":"FAB1","destination_ledger":"3001."},{"source_asset":"FAB1","source_ledger":"3001.","destination_asset":"FAB2","destination_ledger":"3002."}]

UPD
In my opinion, routes and sources as well should be added when broadcast is started (within reloadLocalRoutes). But I never come into the routing-broadcast:start. Is it ok? Do you use broadcast to share connectors info between each other?

@emschwartz
Copy link
Member

The fact that the source address is a URL (http://localhost:3002/node1_ledger) instead of an ILP address indicates that you're using an older version of js-ilp-core and/or js-ilp. Please update to the latest versions and try this again.

@alkopnin
Copy link
Author

alkopnin commented Aug 29, 2016

Thank you, Evan

I used latest version of libs, but still had the errors. While I tried to connect my ledger I figure out:

  • Every Connector has a lot of Clients with their own plugin. Plugin should be set explicit

Default plugin inside the Connector is ilp-plugin-bells. It operates with five-bells-ledger. To fix it I should (1) pass additional field type: name to CONNECTOR_CREDENTIALS params. And (2) put custom plugin along with Connector with name: ilp-plugin-< name >

If I don't do this actions (1 and 2) Connector works with ilp-plugin-bells instead of mine. It's okey if ilp-plugin-bells universal, but it isn't, is it?

  • Pairs have to be currency and only 3 symbols. I used incorrect format FAB1, FAB2 etc.

FAB1, FAB2.. won't work, because Interledger has a length restriction: 3 symbols. Also, Connectors have a backends to retrieve the ratio info. It means that I can't to use the custom pairs.

  • This helped me to go ahead. Now I'm trying to send a payment.

Could you clarify please, how the ilp make a transfer? Who is responsible for transaction delivering? Is it plugin or connector? What exactly plugin should do after "payRequest" (inside the send(transfer))?

My apologies for a long thread and huge number of questions. And very thank you for help in my tries to set up Interledger network

@emschwartz
Copy link
Member

emschwartz commented Aug 31, 2016

Re: plugins. Right now the connector defaults to using the ledger plugin for the five-bells-ledger, because that is what it's most commonly being used for in the moment. That may change in the future, especially if we define a recommended ledger interface (see interledger/rfcs#90), in which case the plugin for that ledger interface would probably become the default.

Where do you see the length restriction on currency symbols? If that exists I'm not aware of it.

Connectors use the "backends" to get rate information so if you want to make it work with custom ledgers/assets you would need to also define a backend that provides the exchange rates.

The way you make an ILP payment is by putting a local transfer on hold to a connector with an ILP Packet attached in the data/memo field. The connector gets the incoming transfer and uses the route builder to create the corresponding destination transfer using the ILP Packet. The plugin turns the standard Transfer object created by the connector into whatever a real transfer looks like on that ledger.

Hope that helps, let me know if you have more questions!

@alkopnin
Copy link
Author

alkopnin commented Sep 5, 2016

Thank you, Evan
Regarding len restriction: take a look here five-bell-connector/src/backends/utils.js - line 19

You would like to make an universal plugin, this is good call. When do you have a plans to do it?

Evan, I'm trying to draw and understand transactions flow diagram. And I still have a lot of questions. How connector holds the transfer, is it has an accounts (wallet addresses) in ledgers? Is client send assets to the connector's account? Do the Connector has a balance (if yes who and when set the initial one)?

Also, is connector has a different plugins to connect to different ledgers at the same time? Or flow is - one connector to one ledger over one plugin?

Your idea looks very attractive and I'm really interesting in it. Thanks for your time.

@emschwartz
Copy link
Member

Thanks. I'll check about the length restriction. 3 letter currency codes come from the ISO 4217 standard, but we probably want to support >3 letter codes.

We don't yet have a specific timeline for the standard ledger API and corresponding plugin. My guess is that it'll end up looking somewhat similar to the five-bells-ledger API, so if you are building an adapter for your ledger(s) you might want to start there.

Regarding transaction flows, have you read through the whitepaper? Senders, receivers, and connectors all have accounts on ledgers. The sending client puts a transfer on hold for the first connector with an ILP packet attached. The connector parses the ILP packet and puts a transfer on hold on the next ledger, and so on all the way to the receiver.

All parties get balances on their ledgers the same way you would get a balance at a bank (depositing) or on the Bitcoin ledger (buying bitcoin from someone else).

A connector connects at minimum two ledgers. It needs one plugin per ledger type. A connector that connects two five-bells-ledgers only needs one plugin. A connector that connects Bitcoin and Ethereum would need one plugin for each of those ledger types.

@adrianhopebailie
Copy link
Contributor

We don't yet have a specific timeline for the standard ledger API and corresponding plugin. My guess is that it'll end up looking somewhat similar to the five-bells-ledger API, so if you are building an adapter for your ledger(s) you might want to start there.

@kopnin you should also look at the proposal for a standard REST API from @diminator here: interledger/rfcs#90

Your feedback would be valuable!

@alkopnin
Copy link
Author

Hi,
@emschwartz I've added sequence diagram. I think it can be helpful for everyone who is works with Interledger. Could you have a look, please.
@adrianhopebailie yes, good call. I can do it.
Guys, maybe you have a slack channel?
interledger_seq_general.pdf
interledger_seq_handshake.pdf
interledger_seq_payment.pdf
interledger_seq_transfer.pdf

@adrianhopebailie
Copy link
Contributor

@kopnin we are using Gitter: https://gitter.im/interledger/Lobby

@emschwartz
Copy link
Member

I think the original issue here was addressed so I'm going to close this now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants