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

fix(UpdateParams/tests) Fix bugs and refine tests to ensure ‘make test’ works #3

Merged
merged 2 commits into from
Mar 5, 2024

Conversation

TimmyExogenous
Copy link
Contributor

@TimmyExogenous TimmyExogenous commented Feb 28, 2024

Description

When we use the command line to set ExoCoreLzAppAddress, the transaction might not be routed correctly because a repeated function is implemented in the withdraw module and the referred module name is misused. This PR deleted the repeat code related to setting ExoCoreLzAppAddress in the withdraw module to fix the bug.
It might be better to merge the deposit and withdrawal modules into one. This also needs to change the code related to precompiles, so we might do it in the future.

The test problem caused by token denominal renaming in the later commit has been resolved. Additionally, all tests have been refined by using a shared BaseTestSuite defined in testutil/util.go. The make test command can be executed successfully after the second commit. In the subsequent development, we can write clean test code by referring the shared BaseTestSuite.

Todo

Copy link
Contributor

@adu-web3 adu-web3 left a comment

Choose a reason for hiding this comment

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

LGTM

@TimmyExogenous TimmyExogenous changed the title fix(UpdateParams) Delete the code related to setting ExoCoreLzAppAddress to fix the bug… fix(UpdateParams/tests) Fix bugs and refine tests to ensure ‘make test’ works Mar 4, 2024
@TimmyExogenous TimmyExogenous force-pushed the develop-fix-updateParam-bug branch from d2e2277 to 3d1990e Compare March 4, 2024 06:17
@TimmyExogenous TimmyExogenous removed the request for review from trestinlsd March 4, 2024 06:18
@TimmyExogenous TimmyExogenous force-pushed the develop-fix-updateParam-bug branch from 3d1990e to 49edbaf Compare March 4, 2024 06:52
@TimmyExogenous
Copy link
Contributor Author

The failing "Protobuf / break-check" workflow can be ignored. It reminds us that some protobuf files in the withdraw module have been deleted, but these files have never been used. So, It's fine to delete them. This check won't fail after It's been merged.

use shared test utils to refine the test codes

fix the problem in tests caused by denom rename
@TimmyExogenous TimmyExogenous force-pushed the develop-fix-updateParam-bug branch from 49edbaf to 3002441 Compare March 5, 2024 07:23
@TimmyExogenous TimmyExogenous removed the request for review from leonz789 March 5, 2024 08:52
@TimmyExogenous TimmyExogenous merged commit eacf9e3 into develop Mar 5, 2024
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants