paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.84k stars 667 forks source link

Incorrect unreserve destination when removing an anonymous proxy #284

Open h4x3rotab opened 2 years ago

h4x3rotab commented 2 years ago

Step to reproduce the issue

  1. A creates an anonymous proxy B
    • A may get 2 DOT locked
  2. A can then send some funds to B and add a normal proxies to B (e.g. grant C the ANY access to B)
    • B may get 1.5 DOT locked
  3. Call remove_proxy to remove A's access to B

Expected behavior

A got its reserved deposit released (2 DOT)

Actual behavior

B got its reserved deposit released (1.5 DOT) by the amount of A's reservation (2 DOT, usually larger than B's reserve for C's ANY proxy), but now A is already removed from the proxy list. So its funds were locked forever.

Analysis

When creating the anonymous proxy, the creator (proxy) pays the reserve deposit, while the deposit of a regular proxy is paid by the real account. However, in the code the both deposits are summed up together and stored in the proxy info. The owner of the reservation is not distinguished. So when we remove the proxy from the original creator, the code assumes the release target is still the real account. As a result, the proxy is removed, but the creator's fund is never released and will be locked forever.

To fix the problem, I suggest to record the owner reservation and the real account reservation separately. So when removing a proxy, we can unreserve the correct amount in the right account.

Probably relevant: paritytech/substrate#9794

bkchr commented 2 years ago

CC @shawntabrizi

DaveG7 commented 2 years ago

real example --> Extrinsic

crystalin commented 2 years ago

+1 This is a real issue because it creates inconsistency in the "reserve" amounts :(