Calculation issue with the bonding shares during the migration

[sergfeldman]

アレクサンダー.eth | U B I Q U I T Y: A bit off topic but we had a calculation issue with the bonding shares that were migrated. Me and several others received far less credit for pool ownership than we deserve. This is a big issue we need solved by 2025 (four year lockup)

We couldn’t diagnose where the miscalculation in the contract code took place (perhaps it was the snapshot data for the migration?)

If you’re looking at things now, let me know if you see anything suspicious with the first six or so positions that were migrated

Mae:

It doesn't look like shares from V1 were taken into account and there was no credit for time spent in V1 when migrating. So if someone entered V2 before you could migrate, that would affect things.

アレクサンダー.eth | U B I Q U I T Y: V1 was only live for like less than a month so I’m not too concerned about time spent. The shares I believe were manually snapshotted and inputted in the deployment call data

We can make a custom bounty for it. It’s not an urgent task but it is important

Mae:

I think I’d take snapshots, then fork mainnet from a block before V2 deployment and try to recreate it

[sergfeldman]

The proposal from @hashedMae

For the bounty, I'm thinking $1000 for 20 hours work and if I don't get it resolved, report with what I did find and go from there with next steps.

[0x4007]

The proposal from @hashedMae

For the bounty, I'm thinking $1000 for 20 hours work and if I don't get it resolved, report with what I did find and go from there with next steps.

Approved

[hashedMae]

I'm leaving for DevCon Sunday, so won't have a chance to dig into this until the week of the 17th.

[0x4007]

I'm leaving for DevCon Sunday, so won't have a chance to dig into this until the week of the 17th.

You have until 2025

[hashedMae]

Ran sims and found that the shares between V1 and V2 after migration were identical https://github.com/hashedMae/v2Investigate

[hashedMae]

investigation sims Math all checks out and I've verified that the snapshot data used for the migration is correct. I can't find any issues with deposit users receiving more rewards than migration users.

[0x4007]

Perhaps its just a math quirk but its unfortunate that we can't explain the unexpected behavior. What do you think are the best next steps?

[hashedMae]

I can't even confirm that there is an issue. May need to look at possible scenarios that could cause things to get out of whack. Maybe if the multipliers or tokens minted per block were changed.

[hashedMae]

ERC20 Address 0x7fe65D99a0998Cdba8e1f749303a467dcf87e815

[0x4007]

I can't even confirm that there is an issue. May need to look at possible scenarios that could cause things to get out of whack. Maybe if the multipliers or tokens minted per block were changed.

I don't recall changing these...However there is a slim possibility we did in a panic when we first discovered the bug right before coding up a fix and deploying/migrating. I guess if you dig through the last transactions on the old bonding contracts you'll be able to find that if that happened.

I hope that DAI is alright! We might end up sticking with it for the foreseeable future because gas fees are about half of USDC/USDT on transfers! https://etherscan.io/tx/0x2e0a0ec4145fb7e1ced863c82d5519fc2ef4baec87e28a08683821730a689a80

[hashedMae]

DAI is fine by me, thank you.

[0x4007]

Just a small detail I want to share but you can see my two positions (one is 27% compared to the larger one) and the smaller one is actually farming faster than my larger one.

The only other variable I can think of is when we manipulated the liquidity in the market (forced price resets by removing liquidity) for when the farmer enters?

[0x4007]

@hashedMae I have a hypothesis can you validate this?

See the section titled "Example Multipliers" on the uAD Token overview.

• The minimum lockup time is 1 week.

• The maximum is 208 weeks.

• The multiplier for 1 week lockup is 1x.

• The multiplier for 208 week lockup is 4x.

I believe that my 100k deposit is credited at 1x, while my 27k deposit is credited at 4x.

I believe that migrated deposits are all credited at 1x automatically, whereas my 27k deposit was not migrated.

[0x4007]

The math for my hypothesis checks out almost exactly so I'm pretty confident that this is it.

[0x4007]

@zgorizzo69 would love to hear your input if any on my hypothesis!

[hashedMae]

This is one of the things I tested for. I don't know why the live would be behaving differently if the contracts are the same

[0x4007]

It may also be useful to run a diff off of the live contracts. We technically had 3 different versions of the staking (bonding) contract

1. "v1"
2. "v2"
3. "v2.1"

And then I believe that we had at least 2 different versions of the bonding share NFTs (could have been three though!)

At any point migrating between these upgrades, the multiplier must have been wiped. It is possible that the latest upgraded version of each is valid, but perhaps the previous versions incurred the damage? Just speculating here.

1. Would you be able to highlight where exactly you did this test so we can scrutinize with @0xcodercrane and @zgorizzo69 and ensure that it indeed covers this issue?

2. Regardless I think that my math checks out and I think it could be very useful for you to take another look into this again with this new information.

[0x4007]

Note for the future: I think that we can implement a solution to this issue by making a script that will

1. Check the time from the user's deposit
2. Verify the math to see if their position is affected.
3. updateBond() to fix the multiplier.
4. Compensate the UBQ owed.
   • There are a few strategies to consider here but ideally we should never directly mint UBQ. Instead we should allow claims over time from a shared treasury that grows over time (we already have UBQ distributed to ubq.eth on every claim.)

https://etherscan.io/address/0x2da07859613c14f6f05c97efe37b9b4f212b5ef5#writeContract

[hashedMae]

1. Would you be able to highlight where exactly you did this test so we can scrutinize with @0xcodercrane and @zgorizzo69 and ensure that it indeed covers this issue?

https://github.com/hashedMae/v2Investigate/blob/main/test/migrateCompare.sol#L119

[0x4007]

1. Would you be able to highlight where exactly you did this test so we can scrutinize with @0xcodercrane and @zgorizzo69 and ensure that it indeed covers this issue?

https://github.com/hashedMae/v2Investigate/blob/main/test/migrateCompare.sol#L119

I tried looking at the test function by myself but it isn't clear to me if you are definitely comparing a 52 vs a 208 week lockup migration.

Can you guys help me confirm this @0xcodercrane @zgorizzo69

[hashedMae]

Lines 121 and 122, it's using the same LP amount and Lockup Term for both. If your original lockup was 208 weeks, but then at migration the time remaining was entered, that would give you a lower multiplier in the new one.

[0x4007]

Yes it will be lower but it shouldn't be that much lower. If you review our conversation you will see that my calculation proves that I'm getting a multiplier for only 1 year when it really should have been closer to 4 years.

[0x4007]

Meeting minutes with @sergfeldman (sergey please feel free to edit this post):

1. prep more hypothesis based on business goal and data collected
   • list of hypothesis and ways to evaluate them and if we achieved (or not) the business goal
2. set upper bounds on budget for research initiative
3. set tighter checkpoints for deliverables aim for 1 week $1k

[0x4007]

Moving to an updated issue.