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(precision): increase amount to test precision #75

Merged
merged 3 commits into from
Dec 16, 2021
Merged

Conversation

zgorizzo69
Copy link
Collaborator

No description provided.

@zgorizzo69 zgorizzo69 added the question Further information is requested label Dec 15, 2021
@zgorizzo69
Copy link
Collaborator Author

yarn test test/integration/Hub/UpdateCurveDetails.ts fails
it doesn't come from the toETH function cos if we remove it the result is the same
and there is no caculation from ts code as we compare what foundry.calculateMeTokensMinted returns and the amount of metoken we actually receive
so it should come from the fee

@zgorizzo69
Copy link
Collaborator Author

zgorizzo69 commented Dec 15, 2021

it is not the fee it is because somehow calculateMeTokensMinted doesn't returns the same thing when I call it before minting and when it is called inside the mint function . My guess is the weighted average is a little bit different because on the time spent when I ask for the calculateMeTokensMinted block timestamp is a little lower than when it is called inside the mint function

@pegahcarter
Copy link
Member

it is not the fee it is because somehow calculateMeTokensMinted doesn't returns the same thing when I call it before minting and when it is called inside the mint function . My guess is the weighted average is a little bit different because on the time spent when I ask for the calculateMeTokensMinted block timestamp is a little lower than when it is called inside the mint function

are you sure? Shouldn't we be on the same block

@zgorizzo69
Copy link
Collaborator Author

it is not the fee it is because somehow calculateMeTokensMinted doesn't returns the same thing when I call it before minting and when it is called inside the mint function . My guess is the weighted average is a little bit different because on the time spent when I ask for the calculateMeTokensMinted block timestamp is a little lower than when it is called inside the mint function

are you sure? Shouldn't we be on the same block

This is weird indeed I double checked only one tx so block height must be the same...

@pegahcarter
Copy link
Member

Try comparing output to BancorPower to see if precision issue still persists

@zgorizzo69
Copy link
Collaborator Author

it is not the fee it is because somehow calculateMeTokensMinted doesn't returns the same thing when I call it before minting and when it is called inside the mint function . My guess is the weighted average is a little bit different because on the time spent when I ask for the calculateMeTokensMinted block timestamp is a little lower than when it is called inside the mint function

are you sure? Shouldn't we be on the same block

This is weird indeed I double checked only one tx so block height must be the same...

it is indeed because block.timestamp is slightly different between when we ask for the foundry.calculateMeTokensMinted and when we mint it when we calculate the weighted average

@zgorizzo69
Copy link
Collaborator Author

WA targetAmount > amount targetAmount:688041225234449977308 amount:1282213392634488692

WA targetAmount > amount block.timestamp:1633906778 startTime:1633816758 endTime:1634421558

targetCurve ==== address(0) meTokensMinted:103501205201497303317

WA targetAmount > amount targetAmount:688041225234449977308 amount:1282213392634488692

WA targetAmount > amount block.timestamp:1633906780 startTime:1633816758 endTime:1634421558

targetCurve ==== address(0) meTokensMinted:103503476229975616198

@zgorizzo69 zgorizzo69 added enhancement New feature or request and removed question Further information is requested labels Dec 16, 2021
Copy link
Member

@pegahcarter pegahcarter left a comment

Choose a reason for hiding this comment

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

automine helper func and usage look good 👍

@pegahcarter pegahcarter merged commit 24a803f into main Dec 16, 2021
@pegahcarter pegahcarter deleted the fix/precision branch January 18, 2022 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants