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

Remove reveal_timeout can not be larger or equal to settlement_timout message #2515

Closed
err508 opened this Issue Sep 18, 2018 · 11 comments

Comments

Projects
None yet
3 participants
@err508
Collaborator

err508 commented Sep 18, 2018

Problem Definition

When opening a channel a user is given the option to choose the settle timeout. The UI denotes: The smallest allowed value is 1 and displays reveal_timeout can not be larger or equal to settlement_timeout if a to small settlement timeout is chosen.
I'd suggest to remove the error message here and display the minimum Settle Timeout as constrained by the reveal_timeout in the channel opening view.

@err508 err508 added the ui label Sep 18, 2018

@kelsos

This comment has been minimized.

Contributor

kelsos commented Sep 18, 2018

@err508 the smallest allowed value is 1 is the hint message displayed below the input field above, which is the balance field. Each error message appears on the bottom of an input field and has a red colored text.

@err508

This comment has been minimized.

Collaborator

err508 commented Sep 18, 2018

@kelsos The Settle Timeout must also be larger than the reveal_timeout of the Settle Timeout of a particular token as required here. This token specific information is available already in the client.

@kelsos

This comment has been minimized.

Contributor

kelsos commented Sep 18, 2018

@err508 Thanks, I will add an extra validation to the field.

@kelsos

This comment has been minimized.

Contributor

kelsos commented Sep 19, 2018

@err508 I am having trouble finding where reveal_timeout is actually exposed in the REST api.
The /tokens returns an array of token contract addresses. Any meaningful information (balance, name etc) is then queried directly from the ERC20 token via web3.js. The /connections api doesn't seem to contain any information about reveal_timeout either.

The message reveal_timeout can not be larger or equal to settlement_timeout seems to originate from the REST api.

The only place I could find reveal_timeout information was in the list of channels.

@andrevmatos

This comment has been minimized.

Collaborator

andrevmatos commented Sep 19, 2018

@kelsos reveal_timeout is channel-specific, not token-specific (afaict), and comes in channels endpoints

@kelsos

This comment has been minimized.

Contributor

kelsos commented Sep 19, 2018

@andrevmatos Yes but if I need to validate the settle_timeout against the reveal_timeout on the channel open dialog I can't use the information from the channels endpoint can I?

@andrevmatos

This comment has been minimized.

Collaborator

andrevmatos commented Sep 19, 2018

No, you can't. reveal_timeout is also "tweakable" by clients, but that isn't exposed. Therefore, we could also expose the default value in config.json, as per #2393 (comment)

@kelsos

This comment has been minimized.

Contributor

kelsos commented Sep 19, 2018

@andrevmatos If we can also expose it along with the other information it would be great 👍

@err508

This comment has been minimized.

Collaborator

err508 commented Sep 19, 2018

If the reveal timeout is tweakable, we only have a lower bound for the settle_timeout input given by the token's Settle_Timeout no? As of here the initiator of a mediated transfer chooses the reveal_timeout and the client only has to check that it is smaller than the Settle timeout for its mediating channel.
We could display an explanation at this point, that a larger settlement timeout is correlated to a higher degree of security (in case of offline time) and a larger amount of mediated transfers/mediation fees, but in turn the tokens are locked for a prolonged time if the user might want to move them away from a channel.

@andrevmatos

This comment has been minimized.

Collaborator

andrevmatos commented Sep 19, 2018

Yes, reveal_timeout is a lower bound to settle_timeout, and both are tweakables, although not exposed in the webUI nor the docs. I'm working on a PR to expose the default values in config.json, and the webui will be able to use this lower bounday.

@andrevmatos

This comment has been minimized.

Collaborator

andrevmatos commented Sep 19, 2018

Ok, there's a problem. The reveal_timeout is actually exposed through the API (both rest and python) but isn't taken into account when opening the channel (#2529). I did PR #2531 to expose the values of these variables through webUI's config.json.
@kelsos , please, make a follow up PR over it to use these values throughout the webui:

  • Default settle_timeout in the open dialog populated from the config
  • Minimum settle_timeout value is reveal_timeout * 2
    config['settle_timeout'] < config['reveal_timeout'] * 2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment