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

Add support for ibc hooks & add unit tests #35

Merged
merged 12 commits into from
Sep 8, 2021

Conversation

thomas-nguy
Copy link
Collaborator

@thomas-nguy thomas-nguy commented Sep 1, 2021

#33 #36

Tested with Solo Machine and it works fine

Todo

  • Add tests

PR Checklist:

  • Have you read the CONTRIBUTING.md?
  • Does your PR follow the C4 patch requirements?
  • Have you rebased your work on top of the latest master?
  • Have you checked your code compiles? (make)
  • Have you included tests for any non-trivial functionality?
  • Have you checked your code passes the unit tests? (make test)
  • Have you checked your code formatting is correct? (go fmt)
  • Have you checked your basic code style is fine? (golangci-lint run)
  • If you added any dependencies, have you checked they do not contain any known vulnerabilities? (go list -json -m all | nancy sleuth)
  • If your changes affect the client infrastructure, have you run the integration test?
  • If your changes affect public APIs, does your PR follow the C4 evolution of public contracts?
  • If your code changes public APIs, have you incremented the crate version numbers and documented your changes in the CHANGELOG.md?
  • If you are contributing for the first time, please read the agreement in CONTRIBUTING.md now and add a comment to this pull request stating that your PR is in accordance with the Developer's Certificate of Origin.

Thank you for your code, it's appreciated! :)

@codecov
Copy link

codecov bot commented Sep 1, 2021

Codecov Report

Merging #35 (3ea70c5) into main (0880b6a) will increase coverage by 18.24%.
The diff coverage is 65.74%.

Impacted file tree graph

@@            Coverage Diff             @@
##            main      #35       +/-   ##
==========================================
+ Coverage   3.26%   21.51%   +18.24%     
==========================================
  Files         13       27       +14     
  Lines       1315     1729      +414     
==========================================
+ Hits          43      372      +329     
- Misses      1271     1324       +53     
- Partials       1       33       +32     
Impacted Files Coverage Δ
app/app.go 3.95% <0.00%> (-0.03%) ⬇️
app/test_helpers.go 0.00% <0.00%> (ø)
x/cronos/types/keys.go 0.00% <0.00%> (ø)
x/cronos/types/messages.go 0.00% <0.00%> (ø)
x/cronos/keeper/evm.go 51.72% <51.72%> (ø)
x/cronos/keeper/ibc_hooks.go 58.82% <58.82%> (ø)
x/cronos/types/contracts.go 65.00% <65.00%> (ø)
x/cronos/keeper/ibc.go 78.18% <78.18%> (ø)
x/cronos/keeper/keeper.go 94.33% <93.75%> (+94.33%) ⬆️
x/cronos/keeper/params.go 72.72% <100.00%> (+72.72%) ⬆️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 464899b...3ea70c5. Read the comment docs.

@thomas-nguy thomas-nguy changed the title [WIP]add support for ibc hooks Add support for ibc hooks Sep 2, 2021
@yihuang
Copy link
Collaborator

yihuang commented Sep 3, 2021

the CI failure could be fixed with running this command to update the gomod2nix.toml file:

nix run -f ./nix gomod2nix -c gomod2nix

@yihuang
Copy link
Collaborator

yihuang commented Sep 8, 2021

gomod2nix.toml is not up to date according to CI failure.

Copy link
Collaborator

@yihuang yihuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM other than the CI failure, need to update the gomod2nix.toml.

This was linked to issues Sep 8, 2021
@thomas-nguy thomas-nguy merged commit 0d6b1d6 into main Sep 8, 2021
@yihuang yihuang deleted the thomas/support-ibc-hooks branch September 8, 2021 05:42
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 this pull request may close these issues.

Problem: CRO conversion feature is not tested Support IBC hooks
2 participants