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

Raiden on Arbitrum: fix timeouts naming convention #1580

Closed
netcriptus opened this issue Dec 16, 2021 · 3 comments · Fixed by #1590
Closed

Raiden on Arbitrum: fix timeouts naming convention #1580

netcriptus opened this issue Dec 16, 2021 · 3 comments · Fixed by #1590
Assignees

Comments

@netcriptus
Copy link
Contributor

As pointed in this and this comments, there's some confusion regarding the naming of timeouts in the Smart Contracts.

There are 2 concepts of time going around, one for absolute timestamp, and another one for number of seconds an event, and the names do not reflect it at all.

Refactor the contracts with better naming, and unifying names that should reflect the same thing.

@andrevmatos
Copy link
Contributor

I agree with this, and I think we should take the time to do these refactoring before our next contracts deployment to Arbitrum. But I think this is more of an issue which needs to be tackled first in the contracts, instead of the client, and therefore should be moved there.

@andrevmatos andrevmatos transferred this issue from raiden-network/light-client Dec 22, 2021
@andrevmatos
Copy link
Contributor

andrevmatos commented Dec 22, 2021

My suggestion would be be _expiration for absolute points in time, and _timeout for relative time deltas, but I don't have strong feelings for any particular naming schema, as long as it's consistent.

I think we should take the time to do this renaming in #1573 , since that's handling exactly this change, so we avoid even polluting commit history with names that aren't descriptive.

@weilbith
Copy link
Contributor

weilbith commented Dec 23, 2021

@andrevmatos and I have discussed this topic now and settled on a scheme.
There are two different concepts of the timing functionality here. All of them specify a relative time value that gets added on top of the current time when an event happens. So for example a time value for settling a channel, that gets added to the timestamp when a channel gets closed.
The individual future timestamp that gets calculated can have two different meanings depending on the context. In the case of the channel settle part, it means the channel becomes settleable after this timestamp has passed. In another case like for withdrawing funds from a channel it means it can be withdrawn until this timestamp has passed. It is our desire to express this significant difference in the code and make it very easy to read and understand without much guessing.

Thereby we ended up with calling these variables by the following concept:

  • *_timeout (e.g. settle_timeout) - relative time value that gets added to event timestamp to get any of the following values
  • *able_until (e.g. withdrawable_until) - absolute timestamp until which it is possible to execute a specific action
  • *able_after (e.g. settleable_after) - absolute timestamp only after which it is possible to execute a specific action

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

Successfully merging a pull request may close this issue.

3 participants