Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
htlcswitch: attributable errors #7139
base: master
Are you sure you want to change the base?
htlcswitch: attributable errors #7139
Changes from 1 commit
b22b1be
e74a13a
ef15be1
f942e03
cba6e93
c16a9f7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming we'll want an encrypter specific interface here, even if it's just a marker one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it useful to define a marker interface in golang other than for documenting this specific field? I think it doesn't actually provide any compiler safety because every type will satisfy the interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think
time.Now()
already returns the a value in nanoseconds (which happens to be the max granularity in go too)Also, given that we will return the hold time in ms and some databases cannot have a max granularity for timestamps of microseconds (I am thinking about postgres) should we truncate to micro/miliseconds here? If not we can always store it as
int64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a reason why I had to do this. I think it was to strip off the monotonic clock reading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the incentives for a router to report back honestly and to not to pass on a short error? Will this node be penalized for reporting? If the node does not get penalized for reporting, any relayer could do this to penalize their neighbor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they pass the short error, the upstream node will replace it by a proper error and the sender will indeed penalize this node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we could export
minPaddedOnionErrorLength
and insert it here. So semantically this would tell us that we have a zero message length and zero padding?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could, although in the rest of the lnd repo we generally calculate that number 260 from
lnwire.FailureMessageLength
and I aligned this new code with that.Semantically we'll indeed communicate zero message with zero padding. But it really does not matter what we put here.
As a future step, we could define a new failure code in BOLT 04 for this, but it won't change much about the penalization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fallback to legacy format is probably fine. If this happens to be an attributable error route, the upstream node will substitute with an attributable error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be added to the spec.