-
Notifications
You must be signed in to change notification settings - Fork 82
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
Staking Rewards Multiplier Issue #752
Comments
Were these |
This comment was marked as off-topic.
This comment was marked as off-topic.
It would be nice, if someone start working on this, provide some input here first and a elaborate some thoughts, that reflects to what pavlovcik has in mind |
Check BondingV2 code; there are some functions related to migration. I'm pretty sure that the users had to self migrate by calling the migration related function(s).
You're better off handling other tasks because this is context heavy and I imagine will be one of the most research heavy/difficult tasks we have open right now (out of hundreds of other open tasks.)
There was some work done already last year which allowed me to narrow down the issue to what is specified. |
I'm reviewing this issue and I believe that function _migrate did not apply the multiplier correctly. based on the method names I can read, it says However anybody who did a direct I dont have a chance now to do a deeper dive in all the methods. Did some math based on my bonding share
I'm not sure if the math is correct it seems a little higher than expected. There is a detail to articulate where, with the lockup, the user is credited with "owning" a larger part of the pool value share, but their withdrawal of course only will allow them to pull out up to what they originally deposited. My stake unlocks on July 20, 2025 which is the deadline for this bounty. Update: at the end of the lockup I will be owed |
I just inspected https://etherscan.io/address/0xdae807071b5ac7b6a2a343bead19929426dbc998#readContract#F1 for id 2 and 32. id 2 is the affected one, values returned are 32 is okay, and values are It appears that updating a stake via _lpRewardDebt would make the owed amount immediately claimable which could be manually set at the end of the four years for the three affected stakes |
im getting an eye on this one coming up |
I suppose one other possibility is that when we redeploy the whole protocol, we might be able to reimburse the difference during the migration, and ensure that the new multiplier is accurate. This could be done manually as it is only on three positions to my knowledge. We can easily derive how much is owed on those positions by figuring out the percentage of elapsed time of the four year stake, from when they started staking.
@rndquu do you have a plan for staking migrations in place? |
It's possible, but still better we mock an implementation that produce the results
Yes, we do |
one problem that we have to look for is that this contract is not upgradeable, i think a plan should be make in advance (which basically yeah is happening on this issue) it's how to make everyone's affected whole, as I am aware that these are the older contracts, these contracts are not upgrade ready, thus another route needs to the abstract |
! action has an uncaught error |
Also, testing a single claim is possible with
which should give
|
I will deploy https://github.com/gitcoindev/uad-contracts/blob/staking-mutliplier-fix/contracts/BondingDebt.sol tomorrow on mainnet (perhaps with slight modifications) |
@gitcoindev Could you double check https://etherscan.io/address/0xa891faAFc46854670aF57F3E7c4241bCe7AbCb32#code? I've accidentally kept the |
@rndquu I double checked the contract, also tests. Keeping isUserClaimed will not do any harm. This should be a fire-and-forget operation after |
@0x4007 Now you should grant Open UbiquityAlgorithmicDollarManager, in the "Write Contract" section execute the
Then we'll distribute missing rewards. There is no need to revoke |
It would be great to encode the necessary context on chain because GitHub issues aren't permanent. Is it feasible to include in my transaction somehow? |
https://etherscan.io/tx/0x6131b3f996a98a24b8321fad498b679dd2bdb8f0c43a6018c7c57a6072461bb1 There were multiple transfers to Just realized there's also pending claims for everybody that seem to be unaccounted for.. |
So what's next steps to watch out for? Do we need to fix the amounts again when the stakes are withdrawn? |
I'm about to do the final disbursement of UBQ and its basically an extra 100% of everybody's holdings so I want to embed the transaction data so that if there are questions later I just tell them to look at the transaction data. |
Finally! The next 'final' batch should be done after bonds expiry. Only then the remaining exact payout amounts will be available. The algorithm will be:
This will fork the blockchain, get the current inflation rate and output missing stake values for the same set of bonds. What was paid / disbursed today, should be subtracted from the amount that will be shown.
|
@rndquu , @molecula451 please add to this if I missed anything, but finally after those months the issue seems to be solved ! -) |
Great work team, yes if pavlovcik is satified, i think the task its done
Encoding like how, a reminder that anything on-chain is gas-costly (even more on mainnet) if you can bookkeep off-chain also works |
I saved the snapshot of this ticket in the internet archive, a.k.a the Wayback Machine. Now the issue is stored for ever. https://web.archive.org/web/20240507181232/https://github.com/ubiquity/ubiquity-dollar/issues/752 The issue link is referenced in the deployed contract, see line 7 in the source code: https://etherscan.io/address/0xa891faAFc46854670aF57F3E7c4241bCe7AbCb32#code#F1#L1 The links provide both a traceable proof stored on chain and the explanation in the Wayback Machine in case GitHub issue is deleted / moved. |
gg @gitcoindev |
Happy to spend a few hundred dollars in gas to save myself weeks of headache explaining myself. |
To clarify I was hoping to include the information in the transfer to the investors. |
+ Evaluating results. Please wait... |
There's an encoding bug so it didn't properly render the other rewards! Here are the logs so that everybody can extract their own permits. Sorry! https://github.com/ubiquibot/comment-incentives/actions/runs/9005100392/job/24739641196 |
It is satisfying to see that the vesting script worked flawlessly, and that this current task went off mostly without a hitch. We fulfilled our legal obligations to get each partner vested at the agreed upon percentage by May 2024! 🎉 The only remarks are that the following are slightly lower according to Etherscan holdings, but this is because they transferred UBQ out to other wallets:
![]() Truncated relevant vesting script output:
|
That was indeed a great team work and I think the most time consuming Ubiquity task of all time (so far -) . I also wondered if permits will be generated fully with such amount of comments. The last remaining thing is that the funding wallet needs to be recharged, the permit collection throws : |
Thank you for letting the financier know. I will handle in the morning because it's late here. I've been a bit disconnected from GitHub in order to minimize context switching from the fundraising and partnerships work. The good news is that I just got off a call with https://powerhouse.inc , who is the most relevant MakerDAO unit for our purposes, demoing our system. They like what we have and we are working towards the same vision from different angles. We're figuring out a good strategy for collaboration. |
Hi there just a quick update. Apologies but I'm still working on moving the funds around! Standby for just a little bit longer! |
@gitcoindev you're good to claim, sorry for the delay. |
Thank you! |
Overview
Bonding v2
.MasterChef v1
there was a bug discovered after a month or so of operation.MasterChef v2
was a rushed fix as the bug was critical, which also ended up having a critical bug. This was only live for about two days.MasterChef v2.1
is what we are using currently on mainnet and seems to be stable.v2
which was only three of us) the multiplier seems to have gotten wiped (my personal 4x seems to have been credited only 1x)MasterChef v1
toMasterChef v2.1
seem to not have been affected.Objective
This bounty is to confirm where/how exactly the multiplier got wiped, verify that these are the only affected parties, and to draft a solution that will make the affected parties whole before they withdraw their stake sometime in 2024.
The solution can be semi-manual as there only seem to be three affected parties. My expectation for the solution is: 1. that we can transfer UBQ to make them current (you should provide the math for this) and then 2. modify their existing bonding shares to include the 4x multiplier with instructions you provide by using the Etherscan writeContract interface.
I would scrutinize MasterChef v2 the closest, because only those who seem to have interacted with it are affected (this is from my memory, I am not the expert at on-chain forensics) as well as Bonding v2 as this invoked the migration function.
Appendix
Contracts
Full list of contracts.
Related Conversation
Originally posted by @pavlovcik in #276 (comment)
Update on math:
At the end of the lockup I will be owed 3,993,849.12 UBQ on the bugged position.
The text was updated successfully, but these errors were encountered: