F/rewards distribution consumer #104
Conversation
maurolacy
requested review from
vitsalis,
SebastianElvis,
KonradStaniec and
gusin13
| @@ -49,4 +49,5 @@ pub struct Params { | |||
| /// expressed as a decimal (e.g. 0.5 for 50%). | |||
| #[derivative(Default(value = "String::from(\"0.1\")"))] | |||
| pub slashing_rate: String, | |||
| pub rewards_denom: String, | |||
There was a problem hiding this comment.
Can this be queried from Cosmos SDK or be retrieved here?
There was a problem hiding this comment.
Fixed. Queried using the query_bonded_denom, which makes sense for consistency with other parts of the code.
That said, rewards denom and bonded denom are not necessarily the same. This is something we will need to address at some point.
| .collect() | ||
| } | ||
|
|
||
| fn distribute_rewards( |
There was a problem hiding this comment.
Does this function distributes commission to FP's address, or this happens somewhere else?
There was a problem hiding this comment.
FP commissions are not implemented yet. I suggest we do it in a follow-up.
| // Delegations by finality provider's public key and staking hash. | ||
| // Last type param defines the pk deserialization type | ||
| pub rev: MultiIndex<'a, (String, Vec<u8>), Delegation, (Vec<u8>, String)>, | ||
| // Delegations by staker's (raw, canonical) address and finality provider's public key. | ||
| // Last type param defines the pk deserialization type | ||
| pub staker: MultiIndex<'a, (Vec<u8>, String), Delegation, (Vec<u8>, String)>, |
There was a problem hiding this comment.
Not sure if I understand these two multi index:
- Why would the key contain both raw and canonical addresses? Probably raw address is good enough as the reward distribution does not care about the account prefix at all?
- What is the definition of
rev? - Why should we use multi index and differentiate key / primary key?
There was a problem hiding this comment.
The key does not contain both, just raw. We use raw addresses because CanonicalAddr does not implement PrimaryKey (a technicality). Perhaps we can simply store the raw key in the Delegations struct instead of CanonicalAddr for clarity. It's also more compact, and so, good for storage savings.
rev is a reverse index of delegations per FP. It can be used to replace the DELEGATIONS_FPS store entirely. Same with the direct key, which can be used to replace the current FP_DELEGATIONS.
There's already a TODO in the code for that. Will address in a follow-up.
|
This is now in a good shape to merge IMO. Please take another look when you find the time. Follow-ups to this (different PRs):
|
Co-authored-by: Runchao Han <[email protected]>
maurolacy
force-pushed
the
f/rewards-distribution-consumer
branch
from
eba2f42 to
2497493
Compare
|
Thanks for the support and comments! Will now work in the follow-ups, to put all this into a really good shape. |
Closes #102, with the variant in which we process all the distribution on the Consumer.
Implements the rewards distribution's second step (over delegators) on the Consumer.
Closely follows the mesh security impl, adapting it to our use case.
TODO: