Skip to content
This repository has been archived by the owner on Apr 8, 2022. It is now read-only.

session implementation and genesis staking #132

Draft
wants to merge 2 commits into
base: staging
Choose a base branch
from
Draft

Conversation

b-yap
Copy link
Contributor

@b-yap b-yap commented Nov 19, 2021

A copy of Substrate's pallet-staking to remove pallet-balances dependency.

For this iteration, here are the following things I focused on:

  1. No storage for nominators
  2. copy all session related traits to increase the session number and the era number
  3. At genesis, get balance from Utxo for staking and initial authorities
  4. get the block production working

A couple of things I made that has to be addressed:

  1. Substrate's pallet-staking uses the total_issuance to do elections and votings. Talking to Anton about it, we didn't really know where to start. So for now, I use the overall stake by all stash accounts and use that as basis.
  2. When performing a lock_for_staking, it is better to call the spend() function of Utxo. But given the problem of signing transactions inside the pallet, I took a shortcut and just inserted directly to the the storage of UtxoStore and LockedUtxos.
  3. The hash, the outpoint. It's a quick thing I did to get the lock_for_staking to work, but it is obviously the wrong way.

^ Aside from those, more things to do:

  1. remove the StakingCount from the Utxo
  2. remove StakingHelper.
  3. remove the locked_utxos in the GenesisConfig of Utxo
  4. implement the lock_extra_for_staking + the other actions
  5. dig in more on the other storages of substrate's pallet-staking that is not important.
  6. make the lock_for_staking in the Utxo work, by connecting to my pallet-utxo-staking
  7. More cleaning.

@TheQuantumPhysicist
Copy link
Contributor

Still haven't reviewed the code, but a few comments about the description from above:

When performing a lock_for_staking, it is better to call the spend() function of Utxo. But given the problem of signing transactions inside the pallet, I took a shortcut and just inserted directly to the the storage of UtxoStore and LockedUtxos.

This is actually the correct way to do it. Going through "spend" is wrong, as spend should only be used by callers from the outside. This is the same issue I discussed with Aaro and Amin about smart contracts. We should never create intermediary transactions to modify the state of the database, but we should directly modify the database to the state we need.

The other correct alternative is to expect the user to create a transaction that will be interpreted by the spend call correctly.

We should never create intermediary transactions to change the state of the UTXO set. Usually people do this in bitcoin forks because they want to be able to rebase on bitcoin and to be able to simplify the security model of spending/unspending. But in our case, we don't have to do any of that as we don't even have a base security model to worry about.

Substrate's pallet-staking uses the total_issuance to do elections and votings. Talking to Anton about it, we didn't really know where to start. So for now, I use the overall stake by all stash accounts and use that as basis.

Well, I don't know what to say about this, but we're (again) using balances for staking. I can't blame you for doing this, but ideally staking would be based on outputs, not balances. But given that we have to mimic pallet-staking without using pallet-balances and not reinvent the wheel, I think that's OK. Using the total stake in the network for the raffle sounds like the correct way to do this as it defines your weight as a staker.

session_keys: Vec<u8>,
value: Value,
) -> DispatchResultWithPostInfo {
let (total, hashes, utxos) = pick_utxo::<T>(&stash, value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really bad from a practical point of view. So we're looping over all existing UTXOs to find some UTXOs for this user, so that we can lock for him. We're acting like a wallet... but not exactly, because we have to loop over everything.

The more practical way is to have the user create the transaction and spend the outputs he thinks are fit to be staked. Then, in the spend function, while verifying, we see what outputs he created, and the user chooses what outputs should be used for staking.

@TheQuantumPhysicist TheQuantumPhysicist marked this pull request as ready for review November 25, 2021 10:48
@TheQuantumPhysicist TheQuantumPhysicist marked this pull request as draft November 25, 2021 10:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants