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

The congestion controller's minimum value is 1, but if the sender sends a prepare packet with a value of 1, it MAY get a T04 error. #513

Closed
aanchal4 opened this issue Nov 11, 2019 · 4 comments

Comments

@aanchal4
Copy link
Collaborator

@aanchal4 aanchal4 commented Nov 11, 2019

If the sender sends a prepare packet with a value of 1, there's a good chance that it will be rounded down (when the path exchange rates are applied) to 0. So the sender will be sending lots of packets but effectively not delivering anything. The following code for error T04.

pub fn reject(&mut self, prepare_amount: u64, reject: &Reject) {
self.amount_in_flight -= prepare_amount;
match reject.code() {
ErrorCode::T04_INSUFFICIENT_LIQUIDITY => {
self.state = CongestionState::AvoidCongestion;
self.max_in_flight = max(
(self.max_in_flight as f64 / self.decrease_factor).floor() as u64,
1,
);
debug!("Rejected packet with T04 error. Amount in flight was: {}, decreasing max in flight to: {}", self.amount_in_flight + prepare_amount, self.max_in_flight);
#[cfg(feature = "metrics_csv")]
self.log_stats(0);
}

Potential solution 1: The reject packet can have a separate error code for such a situation (T05: minimum acceptable amount?), which would mean that the sender should increase/adjust the amount in the packet accordingly.

A) This reject packet MAY indicate the details that the sender can use to compute the path exchange rate. It’s more in the vein of path exchange rate discovery here and the STREAM packets already have that capability. The sender can then adjust the minimum value for the next prepare packet based on:
a) calculated exchange rate from reject packet, and
b) minimum acceptable value for a prepare packet to be fulfilled.

receipt: StreamDelivery {
from: from_account.ilp_address().clone(),
to: destination_account,
sent_amount: source_amount,
sent_asset_scale: from_account.asset_scale(),
sent_asset_code: from_account.asset_code().to_string(),
delivered_asset_scale: None,
delivered_asset_code: None,
delivered_amount: 0,
},

B) If the reject packet does not explicitly indicate the necessary details to compute the path exchange rate information (for some reason), then the sender can slowly start increasing the amount on the packets so as to see when it starts to get the fulfill packets again.
@emschwartz

@emschwartz

This comment has been minimized.

Copy link
Member

@emschwartz emschwartz commented Nov 19, 2019

Interledger allows sending 0-amount packets so I don't think we can or should rely on receiving an error here.

This is tricky because I think the T04 error is being caused by something else (not because the packet has an amount of 0). If the packet were reaching the STREAM server, I would say that the sender should use the prepare_amount in the STREAM response packet to figure out that 0 had been delivered to the receiver. But because something else is throwing a T04 first, it's not getting to the receiver.

🤔

@kincaidoneil

This comment has been minimized.

Copy link
Collaborator

@kincaidoneil kincaidoneil commented Feb 27, 2020

This is tricky because I think the T04 error is being caused by something else (not because the packet has an amount of 0). If the packet were reaching the STREAM server, I would say that the sender should use the prepare_amount in the STREAM response packet to figure out that 0 had been delivered to the receiver. But because something else is throwing a T04 first, it's not getting to the receiver.

This is a pretty bad issue, since if any two peers are configured with low credit limits such they need to settle during a STREAM payment, it will always fail since the congestion controller gets down to 1 and would never be able to ramp back up due to the rounding issue. (In practice, this triggers an R01 since the connector won't forward packets that round down to 0, and the payment fails even faster).

I think there's a neat solution to this:

Since the sender knows their minimum exchange rate and maximum slippage they'll allow for the path, they can calculate a minimum source packet amount such that the minimum destination amount arrives at the recipient without failing due to rounding (of course, it can still fail if the exchange rate is too low). The cool thing about this is if any intermediary takes more slippage such that it rounds down to 0, it wouldn't matter, since it wouldn't meet the minimum destination amount and the recipient would reject it anyways.

In order of precedence, here's how each packet-amount-strategy would fit in:

  1. Remaining amount to send (amount to send minus amount fulfilled and amount in-flight)
  2. F08 maximum packet amount
  3. Minimum source amount based on minimum exchange rate/slippage
  4. Congestion controller maximum remaining in window

So, the minimum source amount would supersede the congestion controller's maximum in-flight amount, but not F08s or the remaining amount to send.

@kincaidoneil

This comment has been minimized.

Copy link
Collaborator

@kincaidoneil kincaidoneil commented Feb 27, 2020

So, how might this minimum source packet amount be calculated?

Here's my model of this:

  • SCALED_RATE — Factor to convert source amount to destination, between scales, and with slippage or a spread. For example, from scale 6 XRP to scale 9 ETH, with no slippage, this factor is ~1.035555 at current rates.
  • MARGIN_OF_ERROR — Minimum percentage difference between the sender's scaled rate and the connector's scaled rate (remember: both already include slippage!). For instance, if the sender and connector use exactly the same rate and slippage, the payment will likely always fail: the connector will round the decimal amount down, and the sender will round the decimal amount up when calculating the minimum destination amount. So, there must be some difference between these two scaled rates in order for the payment to succeed. The larger this difference is, the more "wiggle room" we have, and the more "wiggle room" we have, the smaller we can make the minimum source packet amount without it getting rounded down and rejected since it doesn't meet the minimum destination amount. But, if we set this margin of error or minimum percentage difference too low, we'd have to send massive packets, which would be bad.
  • source_amt * SCALED_RATE = dest_amt
  • MARGIN_OF_ERROR = (ceil(dest_amt) - dest_amt) / dest_amt

Conveniently, this simplified down to this inequality:
source_amt > 1 / (SCALED_RATE * MARGIN_OF_ERROR)

Example:

  • The sender's scaled rate is 1.03555 (scale 6 XRP to scale 9 ETH)
  • The sender's margin of error is 0.0001. This means we're assuming the minimum scaled rate of the connector to be 1.03555 + 0.0001, or 1.03565. (How we might choose this parameter is probably another discussion).
  • Per the inequality above, our minimum source amount is 9656.65..., which we'll round up to 9657. This will be the amount for the packet.
  • We'll also use our same scaled rate to calculate the minimum destination amount, so 9657 * 1.03555, which is 10000.3063, but we'll round up to 10001.
  • Suppose the connector's scaled rate actually is the minimum we're assuming it could be, 1.03565. Now, when the intermediary will compute 9657 * 1.03565, which is.... (drum roll) 10001.272, which they'll round down to 10001. Which means, exactly enough got through to the recipient for them to fulfill the payment -- given these parameters/assumptions.
@sappenin

This comment has been minimized.

Copy link
Contributor

@sappenin sappenin commented Feb 27, 2020

Potential solution 1: The reject packet can have a separate error code for such a situation (T05: minimum acceptable amount?)

FWIW, the Rust connector currently intercepts this condition and responds with an R01 error (see here).

I point this out because this condition (0-value delivered yields packet rejection) can be triggered by any intermediary in the payment path, with any variety of ILP error code.

So, we can't rely on any particular ErrorCode (new or otherwise) to trigger this condition. Instead, I think the solution here needs be an implementation detail of the StreamSender.

kincaidoneil added a commit that referenced this issue Feb 28, 2020
- fix #513: compute min source amount necessary to deliver money to fix T04 issues
- fix #634: distribute dust amounts across remaining packets
- fix #633: only apply T00, T01, F99 to fail-fast behavior
- fix #467: continue on R01, other relative others should be terminal
- increase default slippage to 1.5%
@gakonst gakonst closed this in #635 Mar 25, 2020
gakonst pushed a commit that referenced this issue Mar 25, 2020
- fix #513: compute min source amount necessary to deliver money to fix T04 issues
- fix #634: distribute dust amounts across remaining packets
- fix #633: only apply T00, T01, F99 to fail-fast behavior
- fix #467: continue on R01, other relative others should be terminal
- increase default slippage to 1.5%
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants
You can’t perform that action at this time.