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

Allow user to specify destination expiry duration #57

Open
emschwartz opened this issue Apr 2, 2016 · 13 comments
Open

Allow user to specify destination expiry duration #57

emschwartz opened this issue Apr 2, 2016 · 13 comments

Comments

@emschwartz
Copy link
Contributor

Right now the expiry durations are set automatically. The user of this library should be able to specify how long they expect the recipient to take to fulfill the condition

@emschwartz
Copy link
Contributor Author

@MatthewPhinney I think you ran into this as well

@MatthewPhinney
Copy link
Contributor

I did indeed. Thanks for creating the issue.

@yana-novikova
Copy link

@sentientwaffle will this be addressed by your changes to getQuote in connector?

@yana-novikova
Copy link

also should include source expiry

@emschwartz
Copy link
Contributor Author

Source expiry in universal mode is a function of the destination expiry +
all of the connectors' expiry times. Are expiration dates being used in
atomic mode?

On Sat, Apr 16, 2016 at 12:37 AM, yana-novikova notifications@github.com
wrote:

also should include source expiry


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#57 (comment)

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

@sentientwaffle
Copy link
Contributor

@yana-novikova no

@emschwartz in atomic mode the notary's Case should have an expiry, but not the transfers.

@yana-novikova
Copy link

We want to be able to set source or destination expiry on the getQuote because we use expiry value that gets returned from getQuote for setting expiry on the notary case.

@emschwartz
Copy link
Contributor Author

emschwartz commented Apr 19, 2016

@yana-novikova I'm not sure I understand that. If you're determining the expiry in the first place, why not just pass that to the notary instead of passing the value returned from getQuote?

@MatthewPhinney
Copy link
Contributor

The current RC-ILP flow is:

  1. RC gets quote from connector
  2. RC Uses the five-bells-sender executePayment method to turn this quote into transfers, setup the notary case, execute the transfers

executePayment sets up the case in the notary, and does not allow passing a case expiry. The case expiry is based on the transfer expiry.

One option is to call the connector's getQuote endpoint, then change the expiry on the returned transfers.

Since the connector should be able to handle setting an expiry duration on a quote, it seems like updating the connector to support this would be a better option.

A third approach would be to have RC set up the notary case directly, so that we could completely ignore the transfer expiries. I'd be fine with this approach, but I'd have to run it by the RC team. Even if we took this approach, I think we'd want to change the connector to allow a user to specify the transfer expiry as a query param.

@emschwartz
Copy link
Contributor Author

The sender already handles turning the expiry_duration returned by the connector into the expires_at time for universal mode (here). It should probably do the equivalent for atomic mode here and either use the quote's expiry_duration or more likely a value the user passes in.

I'm not sure I understand the options involving changing the connector or ignoring the expiries (wouldn't the latter mean that the expiries are put in anyway and the transfers expire even when you don't want them to?)

@MatthewPhinney
Copy link
Contributor

We only care about the case expiry. But the only way we can change it, is by changing the expiry_duration of the quotes the connector returns (before we submit these quotes to executePayment).

Changing the connector's /quote endpoint is a separate, but related, task, since the docs say destination_expiry_duration is a valid query param, but this query param isn't actually used (as far as I can tell).

@yana-novikova
Copy link

any update on this?

@sentientwaffle
Copy link
Contributor

Well, interledgerjs/ilp-connector#150 fixes the issue @MatthewPhinney references (destination_expiry_duration being ignored).

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

4 participants