Delete (or deactivate?) transferExpiryMonitor about five-bells-ledger

[emschwartz]

Transfer expiry monitor is needed for universal mode but it shouldn't be
activated for transfers that don't have expiration dates

On Thu, Jan 14, 2016 at 11:44 PM, Chris Clark [email protected]
wrote:

If one of the ledgers goes offline right before receiving the notary's
message to execute and stays offline until after the expiration time, the
sender or connector will lose funds because one escrow will go through and
the other will be cancelled. In atomic mode, we can never let transfers
expire in the ledger; only the notary can process expirations.

If we go with deleting, we can delete transferExpiryMonitor.js,
timerWorker.js, and timeQueue.js; and updateState can be merged into
controllers/transfers.js. Also, accountBalances.js then only depends on
controllers/transfers.js, so it can be moved into a subdirectory of
controllers to follow the weak layering principle. At that point we can
move notificationWorker.js to services to eliminate the "lib" directory.

—
Reply to this email directly or view it on GitHub
https://github.com/interledger/five-bells-ledger/issues/108.

Evan Schwartz | Software Architect | Ripple
[image: ripple.com] http://ripple.com

[sentientwaffle]

Atomic transfers don't have an expires_at set (see https://github.com/interledger/five-bells-sender/blob/feature/dj-atomic-mode/src/sender.js#L105-L112), which means the transferExpiryMonitor will ignore it. No one loses funds if the ledger goes down -- the notary will keep retrying the execution or cancellation, so when the ledger comes back up it will complete the transfer.

[clark800]

The atomic mode spec shows expires_at here and other places: https://docs.google.com/document/d/1EfLo_CocL_a90SKc0EdEUtxKwJZoIPvSH34vCGeAn-A/edit?disco=AAAAAlg09Hc

[sentientwaffle]

I think that is a mistake (in the spec) -- it has a cancellation_condition, so the notary will handle the expiry, not the ledger.

[emschwartz]

That's right. Atomic mode transfers cannot have expiry dates. If they did
some transfers could expire without the others and it wouldn't be atomic
On Jan 15, 2016 7:27 PM, "djg" [email protected] wrote:

I think that is a mistake -- it has a cancellation_condition, so the
notary will handle the expiry, not the ledger.

—
Reply to this email directly or view it on GitHub
https://github.com/interledger/five-bells-ledger/issues/108#issuecomment-172043732
.

[clark800]

Is there any way to identify whether a transfer is intended to be used for atomic mode other than the presence of the "expired_at" field? Looking for validation checks that we could add.

[emschwartz]

The ledger shouldn't actually care, it's just the connectors that care
On Jan 22, 2016 5:31 PM, "Chris Clark" [email protected] wrote:

Is there any way to identify whether a transfer is intended to be used for
atomic mode other than the presence of the "expired_at" field? Looking for
validation checks that we could add.

—
Reply to this email directly or view it on GitHub
https://github.com/interledger/five-bells-ledger/issues/108#issuecomment-174074117
.

[clark800]

I think it's still best to have some redundant checks to try to guard against "user error", where "user" means the client of the ledger API. If someone makes a custom client, they might get confused because expires_at is supposed to go to the notary, but if they copy it to the ledger, which has a similar-looking request, then they have a major failure. If they were forced to explicitly specify atomic mode, we could validate and prevent this failure.

[justmoon]

@clark800 I disagree. First off, Atomic can totally have expiries, it depends on what the client wants to do. If you don't perfectly trust the notaries, you might want to set a n-of-n (full quorum) and set some timeouts in case the notaries don't perfectly agree or one of them goes away. That way you can make sure that the transfers at least get cleaned up. We're not doing that in RC, but it's a perfectly reasonable thing for someone's custom client to want to do.

If you want to do Atomic without expiries, it seems really convoluted to set a "mode": "atomic_no_expiry" field so you can then have the ledger check for you that there is no expiry vs just checking yourself that there is no expiry. Adding a mode field just adds opportunity for more bugs, misunderstandings etc.

I'm not sure you quite realize how many modes are possible. Atomic, Universal, Optimistic, Atomic (multi-notary), Atomic (with timeouts), Tri-phase or Held-Funds-Compensation (HFC), Multi-path, Net settlement / transfer grouping, etc. just to name a few.

The point is that the client's intent is specified in the transfer object. It's much better to have the client define what they want and check it, than for the ledger to guess what the client wants and check it for them.

[clark800]

If you allow the ledger to expire transfers in atomic mode it can lead to loss of funds: one ledger expires a transfer before it gets the message from the notary, but the other ledger gets the message on time and makes the payment. Either the sending RC or the FX connector can lose money permanently (unless there is some out-of-protocol intervention).

Technically, atomic and universal modes are two different protocols, despite having a lot of overlap, which is why it doesn't seem too strange to specify what protocol you are using. @lumberj compared this to versioning: you specify which version of the protocol you are using in a request because different versions of a protocol are actually different protocols in a technical sense.

[justmoon]

If you allow the ledger to expire transfers in atomic mode it can lead to loss of funds: one ledger expires a transfer before it gets the message from the notary, but the other ledger gets the message on time and makes the payment. Either the sending RC or the FX connector can lose money permanently (unless there is some out-of-protocol intervention).

Yes, but that may be preferable than having money stuck in escrow until manual intervention. It depends on your use case.

Technically, atomic and universal modes are two different protocols, despite having a lot of overlap, which is why it doesn't seem too strange to specify what protocol you are using.

Yes, they're different protocols, but they're protocols on a higher layer than the ledger. Would you put a flag in every TCP packet that is part of an HTTP connection to indicate that it's HTTP? No, TCP stacks are supposed to deliver packets, not know about every possible protocol out there.

[clark800]

Yes, they're different protocols, but they're protocols on a higher layer than the ledger.

This makes sense and there is a risk of the ledger acquiring functionality that depends on the protocol field if we were to add it, which would complicate the architecture. And even if it didn't actually acquire different functionality per protocol, it might cause people to wonder about it just because it is possible. So I can see the case that the benefit of extra validation doesn't outweigh these costs.

that may be preferable than having money stuck in escrow until manual intervention

Ok, here's a case I can think of where it would be preferable: if the notary disappears mid-transfer and the ledger operators never respond to requests for manual intervention, then there's a least a chance that the counterparty will be kind enough to refund the money that they got due to failure. Is that roughly the case you were thinking of, or are there better ones?

Closing because there is nothing to do here.