interledgerjs / ilp-connector

Reference implementation of an Interledger connector.
Other
133 stars 59 forks source link

[docs] balance middleware is ignored by plugin #413

Open michielbdejong opened 6 years ago

michielbdejong commented 6 years ago

Looking at https://github.com/interledgerjs/ilp-plugin-xrp-asym-server/issues/11, it seems the settlement is always equal to the prepare amount, regardless of what threshold the admin configures?

sharafian commented 6 years ago

That's because ilp-plugin-xrp-asym-server is a multi-plugin, which means it behaves as a minimal connector. So the connector is basically "peered" with an asym-server, to which the clients connect, rather than the more direct relationship that other plugins use.

michielbdejong commented 6 years ago

ok, I think we should definitely edit the readme of this repo to document that.

michielbdejong commented 6 years ago

Maybe the fact that multi-plugins start to behave as connectors is a sign that we should rethink our software architecture? For instance, deal with BTP and http in the connector, and have plugins only for the ledger-specific sendMoney part?

michielbdejong commented 6 years ago

We discussed this in the research team meeting, and consensus was to just leave things as they are now (but we should obviously still update the readme of this repo to state that balance middleware is not working yet!)

justmoon commented 6 years ago

Yeah, we had a lot of debates about this. Basically, the two options are:

1) Have a connector in the plugin (current way) 2) Explicitly support multi-plugins in the connector

So far, we've said that the connector is already plenty complex and the connector you'd want for multi-plugins has some different behavior anyway, so it's cleaner to keep them separate. Another reason that favors that solution is that mini-accounts is the only plugin where we actually need to implement one of these connectors because all the other multi-plugins just inherit from mini-accounts.

Depending on what issues we run into, we may want to change to option 1 at some point, but right now I still really like the current architecture. ilp-connector is a connector that supports a small number of plugins. mini-accounts is a connector that supports a large number of homogenous accounts. Each can have different functionality due to the different use case. At the same time, mini-accounts can fairly seamlessly plug into ilp-connector.

A lot of the middleware system in ilp-connector would have to be written very differently if it were to support multi-plugins explicitly. And that might make it worse for normal plugins.

What may also play into this decision is what happens when we try to update ilp-connector-shard and create a sharded mini-accounts.

michielbdejong commented 6 years ago

I just actually realized that the multi-plugin's deep packet inspection only works for delivery, not for forwarding. :( Right? Or am I missing something? What would happen if the multi-plugin is not the outgoing connection of the last connector, but of one of the intermediate connectors?

sharafian commented 6 years ago

No, the forwarding vs. delivery distinction is based on whether the plugin ever asserts that something is the last hop (and then sets the transfer amount to match the destination amount in the ILP packet).

The line you link just means that mini-accounts can have children, but neither peers nor parents. So long as one of the clients starts their ILP address with the mini-accounts instance's ILP address, they can have as many further hops as they want.

justmoon commented 6 years ago

I just actually realized that the multi-plugin's deep packet inspection only works for delivery, not for forwarding. :( Right?

Depends on your definition I suppose. We usually took delivery to mean that you couldn't charge fees or change the currency once a connector thought it was delivering, but in the new architecture you can always have your own currency if you want, because quoting is end-to-end and connectors make no assumptions about where they are in the chain.

It's true that mini accounts can only route to destinations whose ILP address starts with its own. That's because the connector inside mini-accounts does not participate in the routing protocol. This makes sense when you think about the use case for mini accounts. Mini accounts exists so that you can provide ILP connectivity to a large number of anonymous clients. You wouldn't trust those clients to publish routes to you anyway because they're anonymous. If you want to give one of them routing access, you can always add them as a peer.

To be clear, it would be totally possible to make a version of mini accounts that has a full routing enabled connector, but the purpose isn't obvious to me. The peers that you want to give routing access to, you already have to hand-select. So you can just add them as individual accounts using ilp-plugin-btp.

michielbdejong commented 6 years ago

You wouldn't trust those clients to publish routes to you anyway because they're anonymous.

Ah right, it does work as long as you don't announce routes "upstream" to a peer that runs a multi-plugin. That restriction is not obvious for newcomers though (it wasn't even obvious for me), so we should probably document that in the readme of this repo.

It also means, for instance, that we can't currently test route updates on the testnet, so we'll have to work something out for that.