Make sure that the error receipt we write always has a sequence greater than the existing one. #5237
Conversation
…t-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
…t-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
…t-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
…te-always-has-a-sequence-greater-than-the-existing-one' of https://github.com/cosmos/ibc-go into cian/issue#5226-make-sure-that-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
chatton
requested review from
colin-axner,
AdityaSripal and
damiannolan
as code owners
…t-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
…t-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
…t-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
…t-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
| @@ -944,3 +944,21 @@ func (k Keeper) restoreChannel(ctx sdk.Context, portID, channelID string, upgrad | |||
|
|
|||
| return channel | |||
| } | |||
|
|
|||
| // WriteErrorReceipt will write an error receipt from the provided UpgradeError. | |||
| func (k Keeper) WriteErrorReceipt(ctx sdk.Context, portID, channelID string, upgradeError *types.UpgradeError) { | |||
There was a problem hiding this comment.
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.
yeah I think we can probably make WriteErrorReceipt unexported and add a wrapper in export_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.
…t-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
…t-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## 04-channel-upgrades #5237 +/- ##
=======================================================
+ Coverage 80.37% 80.39% +0.01%
=======================================================
Files 197 197
Lines 14973 14984 +11
=======================================================
+ Hits 12035 12046 +11
Misses 2474 2474
Partials 464 464
|
…t-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
…t-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
chatton
deleted the
cian/issue#5226-make-sure-that-the-error-receipt-we-write-always-has-a-sequence-greater-than-the-existing-one
branch
Description
closes: #5226
Will wait until #5207 is merged to mark as R4R as the tests will need a small refactor.Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/).godoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.