Hello everyone !
We decided to skip this week's core dev meeting I figured I'd do a report of what I'm working on.
RC delegations
Thanks to some suggestions by @blocktrades' team I rewrote a good amount of the tests, and added some more, while testing I ended up finding a bug that is also affecting the mainnet, luckily the impact is very small. Nonetheless I issued a fix (it's the kind of stuff that could snowball if left unfixed since we are building things on top of it with rc delegations).
Then I spent sooooo much time chasing a bug which actually wasn't one. It turns out it's just kind of bad UX due to the specificities of the testnet, if you are following along the merge request this text will make sense to you, if not feel free to skip that part.
Issue raised by bt's team:
I recognized problem with 'withdraw_vests'. I was created test which was testing possibility of 'withdraw vesting' when almost all of RC is delegated. Is not possible to 'withdraw vests', when We have enough RC to create operation (more then 3 RC) and other part of RC is delegated. Error say that we have 0 RC, but need 3 RC. We think this is bug.
LINK: https://gitlab.syncad.com/hive/hive/-/blob/8ac306a689811a637b7654812bd8b4dd4e13ce5a/tests/functional/python_tests/direct_rc_delegations_tests/test_rc_delegation_behavior.py#L39
My answer after digging:
Okay, it took me forever to understand what was going on here, it's not a bug it's the way the system is setup to work that's a bit counter intuitive if account_creation_fee is 0.
I just changed the value in the tests to delegate all the rc -30 instead of 3, otherwise this assert
assert rc_account_info(accounts[0], 'rc_manabar', wallet)['current_mana'] > 3
wouldn't work, so the test is the same except for that line:
wallet.api.delegate_rc(accounts[0], [accounts[1]], number_of_rc - 30)
In this scenario, right before we do the vest withdrawal the RC state of the user is like so:
vesting shares: 2197
max rc: 2197
delegated_rc: 2167
manabar: 27 (3 rc were used to delegate the rc)
max_rc_creation_adjustment: 0
Then once we withdraw all the vests, we end up at a deficit following get_maximum_rc:
int64_t get_maximum_rc( const account_object& account, const rc_account_object& rc_account, bool add_received_delegated_rc )
{
int64_t result = account.vesting_shares.amount.value;
result = fc::signed_sat_sub( result, account.delegated_vesting_shares.amount.value );
result = fc::signed_sat_add( result, account.received_vesting_shares.amount.value );
result = fc::signed_sat_add( result, rc_account.max_rc_creation_adjustment.amount.value );
result = fc::signed_sat_sub( result, detail::get_next_vesting_withdrawal( account ) );
result = fc::signed_sat_sub( result, (int64_t) rc_account.delegated_rc );
// Used if we want to get the actual amount of RC and not real rc + received rc
if (add_received_delegated_rc) {
result = fc::signed_sat_add(result, (int64_t) rc_account.received_delegated_rc);
}
return result;
}
the result is -139. So this triggers update_outdel_overflow, to go reduce the delegation in order to bring back the amount of rc up to max_rc_creation_adjustment, the problem is that in the testnet, that's 0. So once we want to pay for the cost of withdrawing the vests, we end up with the error message: we have 0 RC, but need 3 RC. It's a bit counter intuitive on testnet environments, but on the mainnet this wouldn't happen as the max_rc_creation_adjustment is not 0. So we would get back to enough RC to execute the operation.
Conclusion:
Anyways, rc delegations are now ready for review again, I am still doing some testing but I am feeling confident now. If you are curious you can follow these details here: https://gitlab.syncad.com/hive/hive/-/merge_requests/245
Additional vops
The community has asked for two new virtual operations to track various things:
https://gitlab.syncad.com/hive/hive/-/issues/187 => A new op for tracking missed blocks, makes it easier for a bunch of alert systems to run
https://gitlab.syncad.com/hive/hive/-/issues/172 => new op to record the current price that was used when converting hive to hbd during a transfer to the treasury.
I am working on that during christmas, I doubt it'll take me very long, I am currently almost done with the missed block one.
You can follow that work there: https://gitlab.syncad.com/hive/hive/-/merge_requests/332
Found the issue which caused the initial testnet to take a minute before starting
Somehow I run into this issue but blocktrades' team doesn't (I know because the python tests are set to timeout after 20 seconds for node initialization and that issue causes the initialization to take at least 60 seconds on relatively high end hardware). For the longest time I thought this was normal but I dug some more and found the root cause to be a bug. I have raised an issue here: https://gitlab.syncad.com/hive/hive/-/issues/210 I will provide more details and potentially work on a fix later on.
A minute may seem trivial, but when you are restarting a node every time you execute a test, it piles up very very quickly, so solving this would help a lot in the long run.
That's about it, thanks everyone and I hope you stay safe & have fun during the holidays :)
If you like what I do please consider voting for the witness I co-manage: @steempress
Hey thanks for the update and the RC work.
Great to see those new ops added! The pricr for the direct hive to hbd conversions when hive is sent to the dhf is great. Can we have some op/record for the actual process ... like 100 hive converted go xxx hbd.
That will most likely be part of the vop :)
Great!
There is a cooldown period when we remove an RC delegation?
The 2 new virtual operations are very interesting.
What do you think about doing something similar every 1,000,000 blocks for the value per MVEST used in the VESTS to HP conversion? It will have an ID for the VOP to filter them in
account_history_api.enum_virtual_ops?Oh and Merry Christmas 🎄
Not really but you don't get the RC back, you have to regenerate it, so if you want to redelegate the same RC, you'll need to wait a week for it to regenerate.
What kind of use would you see for it ? It's not really a good idea to base calculations off it if it's for financial reconciliation/tax purposes since the value may not be exact.
Thanks !
Thanks for your answers, for the last one, it was rather in the case of historical data analysis and the analysis of its evolution
RC delegation tool, we need it more than ever with growth we are seeing at @splinterlands and Where @leofinance is moving to onboard users after mobile app launches. Good to see progress on this.
Thanks, I think soo too, It's designed in a way where we'll be able to quickly delegate to many users so SL/LEO will be able to make all their users benefit very quickly.
If the RC delegation is accepted and passes all the test, will it need to be part of a hard fork, or just a stand alone update? I forget what the answer was a month or so ago.
it's not accepted yet though, it can be standalone but it will be part of hf26 so that we bundle all of our updates and qa as one.
Thank you and Merry Christmas/Happy Holidays. Hive has made a difference in a few lives, and you being part of the Devs has been a part of that difference so your efforts are appreciated.
Thank a lot for your kind words :)
thank you for the effort and update!
RC delegations are very nice!
amazing work, thank you so much!
@tipu curate
Upvoted 👌 (Mana: 61/81) Liquid rewards.
I appreciate your work. Thanks for the update
Wow, what a good job. Merry Christmas
Thanks so much for this great information, merry Christmas and a happy new year in advance to you!!!