refactor: ics27 indicator tests of deterministic error codes and message responses #1349
Conversation
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
colin-axner
changed the title
colin-axner
commented
May 12, 2022
Codecov Report
@@ Coverage Diff @@
## carlos/upgrade-pr-branch #1349 +/- ##
=========================================================
Coverage 79.88% 79.88%
=========================================================
Files 152 152
Lines 10982 10982
=========================================================
Hits 8773 8773
Misses 1783 1783
Partials 426 426
|
colin-axner
requested review from
seantking,
AdityaSripal,
damiannolan and
fedekunze
as code owners
colin-axner
changed the title
crodriguezvega
changed the title
crodriguezvega
approved these changes
May 16, 2022
9 tasks
|
would appreciate a post merge ack from another person cc @seantking @damiannolan The changes are pretty straightforward, but always nice to have 2 pairs of eyes on important changes |
crodriguezvega
added a commit
that referenced
this pull request
May 17, 2022
* deps: upgrade of SDK to 0.46 and tendermint to 0.35 * some changes from review comments * some review comments * refactor: simplify IBC redundant relay check in given restructure of SDK v0.46 (#1288) * refactor: simplify ibc redundancy check used as sdk middleware Instead of having the function checkRedundancy check if the call is on CheckTx or SimulateTx, only call the function on CheckTx or SimulateTx * add godoc * more review comments. * another review comment * remove tests since legacy REST endpoints have been removed in SDK 0.46 * chore: update sdk to v0.46.0-beta2 * refactor: ics27 indicator tests of deterministic error codes and message responses (#1349) * begin refactoring ack_test * fix test due to delayed block execution * refactor: switch ics27 response creation to use new SDK version, update tests * fix import * review comment Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> * review comment. * pass nil context Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> * review comment Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> * remove unused import * remove unused import * fix for race condition in tests * remove replace directive to make it build in pre-monterrey mac OS X Co-authored-by: Carlos Rodriguez <crodveg@gmail.com> Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: billy rennekamp <billy.rennekamp@gmail.com>
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Readjusts the acknowledgement test which ensures the error code is deterministic (and thus can be included in the acknowledgement without halting the network.
In v0.35, tendermint made the construction of the LastResultsHash private. In order to maintain the spirit of the original test which allows us to automatically detect when our expected behaviour changes, I have added in process tests which commit a single transaction via tendermint. I then reconstruct the expected hash and ensure they are equal. To show the code is deterministic, I reconstruct the expected hash again with a different error code and show they are not equal. The same is done for successful transaction (a different message response is used)
I have updated the handling of transaction responses in the ics27 acknowledgement as specified in ADR 003
closes: #XXXX
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.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes