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

x/auth new test - multisig tx on simulate endpoint #9038

Merged
merged 12 commits into from
Apr 5, 2021

Conversation

technicallyty
Copy link
Contributor

Description

This PR adds a test for a multisig tx to the simulation endpoint. Currently there are no tests with multisig tx on simulate endpoint.

Motivation: Issue where developer could not successfully call calculate gas with a multisig due to txFactory shortcomings. This test serves to address that issue by presenting an alternative method to calculating gas with a multisig tx by calling the simulate endpoint.

ref: #8821


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.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@technicallyty technicallyty changed the title x/auth new test x/auth new test - multisig tx on simulate endpoint Mar 31, 2021
Copy link
Contributor

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

pre-approving

x/auth/tx/service_test.go Outdated Show resolved Hide resolved
x/auth/tx/service_test.go Show resolved Hide resolved
@amaury1093 amaury1093 requested a review from cyberbono3 April 1, 2021 15:41
@amaury1093 amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Apr 1, 2021
@amaury1093
Copy link
Contributor

@technicallyty the failing test seems related to the PR

@technicallyty
Copy link
Contributor Author

@technicallyty the failing test seems related to the PR

Turns out one of the other tests was causing the accounts to not exist anymore? I moved the account creation into the test and it works now.

@codecov
Copy link

codecov bot commented Apr 2, 2021

Codecov Report

Merging #9038 (7a43b16) into master (e3a0148) will increase coverage by 0.04%.
The diff coverage is 22.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9038      +/-   ##
==========================================
+ Coverage   58.95%   58.99%   +0.04%     
==========================================
  Files         575      575              
  Lines       32191    32198       +7     
==========================================
+ Hits        18977    18996      +19     
+ Misses      10999    10985      -14     
- Partials     2215     2217       +2     
Impacted Files Coverage Δ
types/module/module.go 68.36% <0.00%> (-5.26%) ⬇️
simapp/app.go 82.75% <100.00%> (-0.09%) ⬇️
types/module/configurator.go 18.51% <100.00%> (+3.13%) ⬆️
x/auth/tx/sigs.go 77.50% <0.00%> (+23.75%) ⬆️

@mergify mergify bot merged commit ccdf896 into master Apr 5, 2021
@mergify mergify bot deleted the ty-8821-calcgas_multisig branch April 5, 2021 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. T: Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants