-
Notifications
You must be signed in to change notification settings - Fork 657
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
Make sure that the error receipt we write always has a sequence greater than the existing one. #5237
Merged
chatton
merged 17 commits into
04-channel-upgrades
from
cian/issue#5226-make-sure-that-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
Dec 13, 2023
Merged
Make sure that the error receipt we write always has a sequence greater than the existing one. #5237
Changes from 12 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
84d6a9c
chore: ensure new error receipts are always greater than the existing…
chatton 68bd689
chore: added unit tests for WriteErrorReceipt
chatton 6434b13
Merge branch '04-channel-upgrades' into cian/issue#5226-make-sure-tha…
chatton 5ee32e9
Merge branch '04-channel-upgrades' into cian/issue#5226-make-sure-tha…
chatton c205130
Merge branch '04-channel-upgrades' into cian/issue#5226-make-sure-tha…
chatton 73151c8
chore: updated tests to use panics instead
chatton e9d9561
Merge branch 'cian/issue#5226-make-sure-that-the-error-receipt-we-wri…
chatton 6f5cd0f
Merge branch '04-channel-upgrades' into cian/issue#5226-make-sure-tha…
chatton 9b2e6d5
Merge branch '04-channel-upgrades' into cian/issue#5226-make-sure-tha…
charleenfei f39e203
Merge branch '04-channel-upgrades' into cian/issue#5226-make-sure-tha…
crodriguezvega 1446cc1
Merge branch '04-channel-upgrades' into cian/issue#5226-make-sure-tha…
damiannolan e0075d6
fix: compiler error
damiannolan faf7599
Merge branch '04-channel-upgrades' into cian/issue#5226-make-sure-tha…
charleenfei 190d568
Merge branch '04-channel-upgrades' into cian/issue#5226-make-sure-tha…
chatton d73b138
chore: address PR feedback
chatton 8adcfb4
Merge branch '04-channel-upgrades' into cian/issue#5226-make-sure-tha…
chatton a761b17
Merge branch '04-channel-upgrades' into cian/issue#5226-make-sure-tha…
chatton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
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.
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.
Question regarding this new method, is the intension to make
SetUpgradeErrorReceipt
private? seems a little off to have many functions exposed for this.I was looking at the cancellation handler and also thought that it is not super clear that an error receipt is set within the
restoreChannel
func above this method. It might be worth while opening another issue to see if we could improve that.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.
yeah I think we can probably make
WriteErrorReceipt
unexported and add a wrapper inexport_test.go
, and potentially just remove the writing of the error receipt from the restore fn, and just call it separately. the error receipt is kind of like a side effect of restoring the channel so I'd be happy with that, will create an issue for it.