make transfer.from and transfer.ledger optional in LPI sendTransfer.

[michielbdejong]

When calling plugin.sendTransfer(transfer), we could help the implementer a bit by

• defaulting transfer.from, when omitted, to plugin.getAccount()
• defaulting transfer.ledger, when omitted, to plugin.getInfo().prefix

Also: defaulting noteToSelf and custom, when ommitted, to {} (we discussed before that this was already how the spec text was intended).

[michielbdejong]

The example in the rfcs leaves out from, ledger, ilp, and custom, of which (according to draft 7) only custom is really optional.

While writing tutorial code, I found it's really silly that you have to include from and ledger. Since they are already optional according to the RFC example, can we agree to make them default to plugin.getAccount() and plugin.getInfo().prefix, respectively?

I would also vote in favor of making ilp optional, because

1. that's what we have for Message,
2. even if a connector will reject requests if they don't carry an ilp payment packet, just like when it carries a corrupt ilp payment packet, it does not really make sense for the ledger to have an opinion about this opaque field,
3. there are cases where plugin.sendTransfer is useful without the ilp packet, namely if you expect the receiver to know the fulfilment.
4. it makes the first tutorial simpler ;)

If we decide to deprecate more fields, we should first update all known plugins, and merge the change to the rfcs repo after it's changed in the code everywhere.

[sharafian]

I believe these should be issues on payment channel framework; if they're not optional then that's a bug

[michielbdejong]

if they're not optional then that's a bug

I think you mean that these fields should be optional in the implementation of ilp-plugin-pachafra, even if they are not optional in the spec.

I'm not against that, but this issue (on the rfcs repo) goes one step further and says we should also make this change in all other known plugins, and then write about this in the spec.

[sharafian]

All other plugins already should make those fields optional. But you're right, if they weren't marked optional in the LPI they should be.

[michielbdejong]

We should add unit tests for it to all known plugins before proposing this change.

[michielbdejong]

This proposal made it into the lpi2 experiment - I guess we'll just leave them required for now, and make them optional as soon as someone makes another breaking change to the lpi.

It does make sense that a plugin will not read the from and ledger fields from a sendTransfer call, although the plugin SHOULD fill in these fields when emitting an event.