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

[docs] balance middleware is ignored by plugin #413

Open
michielbdejong opened this issue Jan 31, 2018 · 9 comments
Open

[docs] balance middleware is ignored by plugin #413

michielbdejong opened this issue Jan 31, 2018 · 9 comments

Comments

@michielbdejong
Copy link
Contributor

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

@sharafian
Copy link

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
Copy link
Contributor Author

michielbdejong commented Feb 2, 2018

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

@michielbdejong michielbdejong changed the title balance middleware is ignored by plugin? [docs] balance middleware is ignored by plugin Feb 2, 2018
@michielbdejong
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

michielbdejong commented Mar 4, 2018

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
Copy link

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
Copy link
Contributor

justmoon commented Mar 4, 2018

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
Copy link
Contributor Author

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.

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