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

test(x/wasm): add integration tests for dynamic call #590

Merged
merged 6 commits into from
Jul 19, 2022

Conversation

loloicci
Copy link
Contributor

@loloicci loloicci commented Jul 11, 2022

Description

This PR adds integration tests for

  • sanity dynamic call flow
  • fails to try reentrancy
  • using query and dynamic call together to x/wasm.

Other than them, this PR includes

  • upgrade depending wasmvm to the temporary latest version (it is needed to renew before release)
  • upgrade testify to use ErrorContains

Motivation and context

Finschia/cosmwasm#186 and Finschia/cosmwasm#191

How has this been tested?

These are integration tests themselves.

Checklist:

  • I followed the contributing guidelines and code of conduct.
  • I have added a relevant changelog to CHANGELOG.md
  • I have added tests to cover my changes.
  • I have updated the documentation accordingly. (not needed)
  • I have updated API documentation client/docs/swagger-ui/swagger.yaml (not needed)

@loloicci loloicci changed the title test(wasm): add integration tests for dynamic call test(x/wasm): add integration tests for dynamic call Jul 11, 2022
@loloicci loloicci self-assigned this Jul 11, 2022
@loloicci loloicci marked this pull request as ready for review July 11, 2022 11:36
@codecov
Copy link

codecov bot commented Jul 11, 2022

Codecov Report

❗ No coverage uploaded for pull request base (dynamic_link@3cd586f). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##             dynamic_link     #590   +/-   ##
===============================================
  Coverage                ?   53.16%           
===============================================
  Files                   ?      642           
  Lines                   ?    67269           
  Branches                ?        0           
===============================================
  Hits                    ?    35764           
  Misses                  ?    28562           
  Partials                ?     2943           

assert.Equal(t, len(res.Events), 3)
assert.Equal(t, "wasm", res.Events[0].Type)
assert.Equal(t, len(res.Events[0].Attributes), 6)
assertAttribute(t, "contract_address", "link10pyejy66429refv3g35g2t7am0was7yaducgya", res.Events[0].Attributes[0])
Copy link
Member

Choose a reason for hiding this comment

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

Where does this address, link10pyejy66429refv3g35g2t7am0was7yaducgya, come from? And is this address always the same?

Copy link
Member

Choose a reason for hiding this comment

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

And this address seems to be repeating. If so, how about changing to a const property?

Copy link
Contributor Author

@loloicci loloicci Jul 13, 2022

Choose a reason for hiding this comment

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

The contract address is always the same because the code id and the instance id are the same.

https://github.com/line/lbm-sdk/blob/a18b7c8c01a77e5a1290a0a4600feda80ad3abce/x/wasm/keeper/keeper.go#L878-L882

But, it is out of this test, so I will delete this assertion.

assertAttribute(t, "returned_pong_with_struct", "hello world 101", res.Events[0].Attributes[2])
assertAttribute(t, "returned_pong_with_tuple", "(hello world, 42)", res.Events[0].Attributes[3])
assertAttribute(t, "returned_pong_with_tuple_takes_2_args", "(hello world, 42)", res.Events[0].Attributes[4])
assertAttribute(t, "returned_contract_address", "link18vd8fpwxzck93qlwghaj6arh4p7c5n89fvcmzu", res.Events[0].Attributes[5])
Copy link
Member

Choose a reason for hiding this comment

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

ditto

x/wasm/dynamic_link_test.go Show resolved Hide resolved
@loloicci loloicci requested a review from zemyblue July 14, 2022 06:44
ykoyote pushed a commit to ykoyote/lbm-sdk that referenced this pull request Jul 15, 2022
* More keeper tests

x/wasm/keeper tests are extended to test various input validation.
Keeper input is validated before passing to the keeper method when used
within wasmd application. We cannot ensure such validation when this
keeper is used outside of wasmd application. To keep it safe, fully
validate keeper methods input.

hackatom.wasm is loaded into memory during initialization to avoid
reading file in each test separately. Once migrated to go 1.16, embed
package should be used instead.

Run goimport on certain files.

Some comments fixed or removed.

* ensure that creator address is not nil
Copy link
Contributor

@shiki-tak shiki-tak left a comment

Choose a reason for hiding this comment

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

LGTM

@loloicci loloicci merged commit c44a9c9 into Finschia:dynamic_link Jul 19, 2022
@loloicci loloicci deleted the testing-query-dynamic branch July 19, 2022 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants