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

Adding Fee Calculation Tests #916

Merged
merged 30 commits into from
Apr 16, 2024
Merged

Conversation

idky137
Copy link
Contributor

@idky137 idky137 commented Apr 10, 2024

Adds tests for fee calculation.

@idky137 idky137 marked this pull request as ready for review April 10, 2024 11:00
@idky137 idky137 marked this pull request as draft April 10, 2024 11:57
@idky137 idky137 marked this pull request as ready for review April 10, 2024 16:00
Copy link
Contributor

@fluidvanadium fluidvanadium left a comment

Choose a reason for hiding this comment

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

i dont know why the tests are running as if using the zip317 feature.

zingo-testutils/src/lib.rs Outdated Show resolved Hide resolved
@idky137
Copy link
Contributor Author

idky137 commented Apr 11, 2024

Ive removed the feature gate for fee calculation and commented out zip317 expected_fee, it seams that CI runs tests with all features enabled so these tests wont pass.

When zip317 lands these tests will fail. The current expected_fee will need to be removed and zip317 expected_fee will need to be un-commented, but no new code needs to be written. Ive added comments to show this.

Should now be ready to merge.

change made in [8719fad]

zingo-testutils/src/lib.rs Outdated Show resolved Hide resolved
zingo-testutils/src/lib.rs Outdated Show resolved Hide resolved
zingo-testutils/src/lib.rs Outdated Show resolved Hide resolved
zingo-testutils/src/lib.rs Outdated Show resolved Hide resolved
zingo-testutils/src/lib.rs Show resolved Hide resolved
Copy link
Contributor

@fluidvanadium fluidvanadium left a comment

Choose a reason for hiding this comment

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

tested and acknowledged. this is a good failsafe

Copy link
Contributor

@Oscar-Pepper Oscar-Pepper left a comment

Choose a reason for hiding this comment

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

as we found, implementing a better fee calculation is going to be a large task and I think this is a good test for verifying zip317 for now.

@Oscar-Pepper Oscar-Pepper merged commit 907876e into zingolabs:dev Apr 16, 2024
17 checks passed
@idky137 idky137 deleted the dev_poc_tests branch October 9, 2024 13:01
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.

4 participants