radicle-dev / radicle-registry

An experimental Substrate implementation of the Radicle Registry 📒
https://registry.radicle.xyz
GNU General Public License v3.0
33 stars 4 forks source link

runtime: Design locked deposits #569

Closed NunoAlexandre closed 4 years ago

NunoAlexandre commented 4 years ago

Part of #530, we need to design how the deposits will work in regard to the locked deposits.

A deposit will be locked when an entity is registered in the registry: an org, a user, a new org member, a project, etc.

Requirements:

0. When locking a deposit, we want to lock the current amount specified for the specific type of transaction (x). This means that x is taken from the free balance of the appropriate account.

1. When unlocking the deposit, we want to unlock the exact amount initially locked (x).

2. Safeguard any outwards transfer against the locked amount For instance, if for an account A free has 100 but locked has 80, a transfer of 30 from A to B can't take place

Solution / Discussion

My idea was to have record-based storage for locked deposits. The overall goal with this idea is to keep track of the initially locked deposit amounts, allowing us to meet requirement 1.

So something like a HashMap<AccountId, HashMap<TxType, Vec<Deposit>>> where Deposit { amount: Balance, date: Date } and enum TxType { RegisterOrg(Id), RegisterUser(id), AddMember(Id) }`.

The inner hashmap would be useful to lookup deposits by their type in O(1) without having to O(n) through all the deposits to find the right one and find its amount. Open for discussion.

When a deposit is unlocked, that entry would be deleted.

The locked balance of an account would be calculated and not simply looked up.

In a call with @cloudhead, the point about storage costs and costs of storage reads was brought up and it is an important one. @cloudhead suggested, instead, to have something like HashMap<AccountId, LockedBalance> where LockedBalance = Balance or, preferably, HashMap<AccountId, HashMap<TxType, LockedBalance>>. However, neither of these alternatives seem to provide enough granularity to meet requirement 1..

geigerzaehler commented 4 years ago

For instance, if for an account A free has 100 but locked has 80, a transfer of 30 from A to B can't take place

I find it easier to reason about locked funds if you actually transfer them. For example if you create a project the deposit would be removed from the free balance and a deposit is created. In you example the free balance would just be 20 and 80 are locked up.

So something like a HashMap<AccountId, HashMap<TxType, Vec<Deposit>>> where Deposit { amount: Balance, date: Date }

Why do we have the date there?

1. When unlocking the deposit, we want to unlock the exact amount initially locked (x).

Who initiates the unlock? Is it always the user that provided the deposit? What data is available to the runtime when a deposit is unlocked? That basically tells you what the keys to the deposit storage are.

So something like a HashMap<AccountId, HashMap<TxType, Vec<Deposit>>>

To avoid the double indirection and make access faster I’d suggest the storage map (AccountId, TxType) => Vec<Deposit>.

Why are there multiple deposits associated with one resource (say a specific org) and account?

NunoAlexandre commented 4 years ago

For instance, if for an account A free has 100 but locked has 80, a transfer of 30 from A to B can't take place

I find it easier to reason about locked funds if you actually transfer them. For example if you create a project the deposit would be removed from the free balance and a deposit is created. In you example the free balance would just be 20 and 80 are locked up.

yes, indeed. I had left a commented on #566 that I have deleted in the meantime commenting on this. In sum, I also think the locked values should be withdrawn from the free balance.

So something like a HashMap<AccountId, HashMap<TxType, Vec<Deposit>>> where Deposit { amount: Balance, date: Date }

Why do we have the date there?

I didn't mean to make these sketches correct-proof, so keep that in mind. The date was just an idea that came after a comment around punishing users who would register an unregister entities in a very short period of time, retiring ids thus affecting the registry negatively. By having the date we would know when something was registered and we could implement such behaviour.

1. When unlocking the deposit, we want to unlock the exact amount initially locked (x).

Who initiates the unlock? Is it always the user that provided the deposit? What data is available to the runtime when a deposit is unlocked? That basically tells you what the keys to the deposit storage are.

It's always a user initiating the unlock, but not necessarily the same user. User A might register org O but user B might be the one unregistering it.

So something like a HashMap<AccountId, HashMap<TxType, Vec<Deposit>>>

To avoid the double indirection and make access faster I’d suggest the storage map (AccountId, TxType) => Vec<Deposit>.

Sounds good to me. The proposed alternative was to more smoothly support the needs of the frontend that shows the locked deposits per tx type but we have all the freedom of course.

Why are there multiple deposits associated with one resource (say a specific org) and account?

Do you mean why, in the sketch above, we store Id in the TxType variants?

geigerzaehler commented 4 years ago

... but not necessarily the same user. User A might register org O but user B might be the one unregistering it.

If that’s the case how do you intend to look up the deposit if it’s indexed by the account ID of the user that created the deposit? ((AccountId, TxType) => Vec<Deposit>). It seems more reasonable to have it indexed by the resource ID and get the account as part of the lookup.

Why are there multiple deposits associated with one resource (say a specific org) and account?

Do you mean why, in the sketch above, we store Id in the TxType variants?

I meant why do we associate Vec<Deposit> with a resource instead of just Deposit?

The date was just an idea that came after a comment around punishing users who would register an unregister entities in a very short period of time, retiring ids thus affecting the registry negatively. By having the date we would know when something was registered and we could implement such behaviour.

I understand. It sounds like the vandalism prevention mechanism still needs to be thought out though. We can always add another field later.

NunoAlexandre commented 4 years ago

... but not necessarily the same user. User A might register org O but user B might be the one unregistering it.

If that’s the case how do you intend to look up the deposit if it’s indexed by the account ID of the user that created the deposit? ((AccountId, TxType) => Vec<Deposit>). It seems more reasonable to have it indexed by the resource ID and get the account as part of the lookup.

Yeah, you are right. We could have the org registration deposit associated with the org's account but that would mean that 1) users wouldn't pay the deposit for creating orgs, 2) an org would start with a negative balance, and 3) that someone would profit from unregistering an org. We need to put much better thoughts into this.

I understand. It sounds like the vandalism prevention mechanism still needs to be thought out though. We can always add another field later.

Sure, I just posted it as pseudo-code to sketch an idea.

CodeSandwich commented 4 years ago

We've ditched the deposits.