RC delegation testing report

in #core3 years ago (edited)

image.png

Hi, I must say it's been quite an eventful week, I've basically been scrambling to do as much testing as possible and get everything done in time, we are not out of the woods yet in terms of "ready for the code freeze" but we are in a good shape.

I focused my efforts on writing automated tests in python, because it's the closest to what the actual execution will be like, c++ unit tests are great because you can do a lot of things (like tinker with the blockchain data directly) but I wanted to take a "black box" approach, that way I could test both the apis and the core logic at the same time.

I'll cut to the chase: Rc delegations are looking really solid. Overall I found one bug, and one flaw, as I was testing I did a few tweaks here and there that I think didn't make sense in the original design. For instance being able to delegate from an empty pool, or delegating more max_rc than there is in the pool.

The bug was a problem in the way rc delegation to a pool were calculated, if the pool had 500 rc delegated from someone else, and you delegated 100 to it, it would go to 100 instead of 600. the calculation was not so straightforward so that's how it slipped through.

Then I realized there was a flaw that allowed someone to "draw" rc from a pool, basically here's the flaw:
account: 30/100 rc
pool: 500/500 rc
account delegates 80 rc to the pool
account: 0/20 rc
pool: 530/580

Then the account powers undelegates to the pool, since we don't keep track of how much current_rc was delegated, he will get 80 rc back even though he only delegated 30 (out of 80 max rc).

I thought of keeping track of how much current_rc was delegated to give back but it's a non negligible scope change and I don't want to risk it, so instead I'm forbidding delegating more than your current_rc that way in this scenario the account can only delegate 30 rc and will get 30 (or less if the pool dried up) back. We may revisit this later on but I think it's best to limit the edge cases as much as possible, especially those that may result in the creation of rc which would be disastrous for the chain if found by a spammer.

Tests and work done

Apart from that I updated some api endpoints to make make more sense (still quite a bit of work needed there, there is a lot of confusion between rc, mana, drc, vests as rc etc, this is due to some artefacts from smts where rc delegations was initially made to support smts)

here's a list of all the test that I automated, there is a lot of stuff there. but I know I'm missing some more edge cases, please do comment any ideas that would break it that comes to mind :)

  • happy path (done both via beem and via the cli to validate the wallet):
    • delegate to pool with two different accounts
    • set account creator slot )
    • delegate from a pool to an account
    • use some delegated rc from the account
    • set slot to another account removes the delegation to the slot
  • Set slot delegator
    • try to set the creator slot when the signer is not the user or the creator
    • try to set the recovery slot when the signer is not the user or recovery partner
    • try to set the user slot when the signer is not the user
    • try to change a slot that doesn't exists (there are 3 slots, try to set the fourth)
    • try to set a slot of an account that doesn't exists
    • try to set a slot on an account that exists but sign with an account that doesn't
    • try to set slot 2 to point to user1 when the account already has slot 0 configured to point to user1
    • set slot to another account removes the delegation to the slot again
    • Try to set the account on the same slot twice
  • Delegate to pool
    • try to delegate to an account(pool) that doesn't exists
    • try to remove a nonexistent delegation (delegate 0)
    • try to remove a delegation when the pool doesn't exits
    • delegate to the pool (happy path)
    • try to delegate your max_rc (you can only delegate your current_rc)
    • try to delegate 999999999999999999 rc (way more than what we have)
    • test removing the delegation to a pool while the pool is delegating to someone
      • test that the account benefitting from the pool delegation can use rc before removing the delegation
      • test that the account benefitting from the pool cannot use rc anymore once the pool is dried up
    • test that when an user consumes rc, the delegation rc is used as well as the pool rc
    • test changing a delegation to a pool
      • delegate 800 then change it to 400
      • delegate 100 from another account (the pool should be at 500)
      • undelegate from the account that delegated 400 (the pool should be at 100)
    • test changing the max rc of a pool when you don't have enough current rc (not the same as an initial pool creation)
    • test removing the delegation to a pool with no RC (works, because undelegating gives you back rc so even if you start with 0 rc, you end up with enough rc to pay for the tx)
  • delegate from pool
    • try to delegate from a pool where the user doesn't have a slot set to receive from that pool
    • try to delegate more rc than there is in the pool
    • try to use rc with an empty pool
    • test delegating all the rc in the pool
    • test using some rc while all the rc from the pool is delegated to one user
    • test overdelegating (the pool has 100 rc, we delegate 100 drc to 4 accounts)
    • test using rc from multiple accounts while overdelegated (all use from the same pool)
    • test that an user with rc from his hp that receives a delegation from a pool uses rc from his account and not the pool
    • test reducing the delegation to an account
    • test removing the delegation to an account
    • test increasing the delegation to an account
    • test delegating from a pool when the pool has a deficit (if the pool has 20 current_rc and we delegate 30, the current_rc on the account should be 20)
  • test exploiting to print rc (delegate all the rc to a pool, delegate to yourself, use the drc, then undelegate to the pool, you shouldn't get more rc back)

If you want to follow along here's the MR: https://gitlab.syncad.com/hive/hive/-/merge_requests/164

the beem tests are here: https://gitlab.syncad.com/hive/hive/-/blob/27e5eacc0021cf4bf22c1b3d0be4c7e47d6eb08c/tests/functional/python_tests/rc_tests/beem_rc_tests.py

Support what I'm doing

If you like what I'm doing, please consider voting on my new proposal:
https://peakd.com/proposals/167
hivesigner

Sort:  

Not directly related to the tests but a question... will there be a way to see how much RC accounts have been using over a period of time like 24hrs or 7 days? Some sort of api?

This can perhaps help rc delegations perhaps identify possible abuse.

Good question! Sadly no, storing that kind of fluctuation would cost way too much in space to be done in L1. But could be done via L2 in an external db that tracks all the transactions and calculates each time. But it's gonna be quite a challenging project.

I think to identify abuse you could look every say, 5-6 hours and check which are the accounts with low rc out of all the ones you delegated to, and if one dude is always at 0 current rc that means he's either very active using a ton of rc or an abuser.

can only delegate 30 rc and will get 30

Is undelegation from RC pools instant? If it takes 5 days like HP, then it makes sense to give back the full amount because RCs should 100% recharge over 5 days.

It's instant, the 5 days HP limitation is linked to duplicating voting power, which doesn't exists via RC

(use all your voting power, delegate to someone else, use all your voting power again, undelegate, delegate to someone else etc etc)

I see, thanks.

Although, the voting power problem could be solved in the same way as RCs then, much better to get back the HP and allow a powerdown to start without having any voting power then waiting 5 days.

I thought of keeping track of how much current_rc was delegated to give back

I guess this is the best solution and applying that to HP too would be really good if it doesn't cause worse performance.

You can't do it the same way with voting power because the current voting power is not correlated with your hive power. someone at 50% voting power with 500 hp != someone with 50% vp at 1000hp

I guess this is the best solution and applying that to HP too would be really good if it doesn't cause worse performance.

I think so too, but it will be there for hf26. I think it's good to ship an mvp version and improve on it later on.

good work! can this be integrated into the next HF?

That's the plan

thank you ! Not confirmed yet but it's starting to look good

Loading...

Please I want to no more about it