[DRAFT] SV weight management fully on-ledger#5567
Conversation
Signed-off-by: Jose Velasco <jose.velasco@intellecteu.com>
| choice DsoRules_AddSvOperator : DsoRules_AddSvOperatorResult | ||
| with | ||
| newSvParty : Party | ||
| -- DRAFT note for the reviewer: In the future, newSvName should not be used, since an SV operator may operate multiple SVs. |
There was a problem hiding this comment.
I would suggest making this choice
AddSvNode, not AddSvOperator, and if a single operator operates multiple nodes, then it is just listed as the operator on each of them.
There was a problem hiding this comment.
ok
On the other hand:
- What do you think about my comment on
newSvName : Text? - What about
newSvParty : Party? Actually it is an SV operato even though we insert it in thesvsMap,
There was a problem hiding this comment.
I think we will still need a name for sv nodes, at least for onboarding, so probably need a newSvNodeName.
we insert it in the svs Map
Yeah, that's an ugliness we'll probably need to carry with us as we cannot rename that map. Ideally, I'd rename it to svNodes, but that's an incompatible daml change.
There was a problem hiding this comment.
I would suggest making this choice AddSvNode, not AddSvOperator, and if a single operator operates multiple nodes, then it is just listed as the operator on each of them.
I'm afraid the current model cannot support a single SV operator party operating multiple nodes, because SV operators are used as keys in the map.
svs : Map.Map Party SvInfo
There was a problem hiding this comment.
I think that actually makes sense to forbid.
The operator party is an internal party in the node itself, so clearly they should be unique per node.
| -- which is not a requirement in the current proposal. | ||
| newSvName : Text | ||
| newSvParticipantId : Text | ||
| joinedAsOfRound : Round |
There was a problem hiding this comment.
I don't think we need this, this was typically to track rewards. If rewards are no longer associated with the node, we don't need it in the node contract
There was a problem hiding this comment.
Currently, joinedAsOfRound is part of SvRewardState, which ensures that SV rewards are created only once per round.
-- check and update state
rewardState <- fetchAndArchive (ForSv with dso; svName = info.name) rewardStateCid
let state = rewardState.state
require
("Round " <> show openRound.round <> " is greater than the last round a reward has been received for " <> show state.lastRoundCollected)
(state.lastRoundCollected < openRound.round)
There was a problem hiding this comment.
Right, but this is for adding a node, so no weights associated with it.
Or do we need it initially during the migration period?
There was a problem hiding this comment.
I'm not referring to the migration period.
My point is that the SV operator exercising the new DsoRules_ReceiveSvRewardCoupon_V2 still needs to provide its SvRewardState contract, which contains lastRoundCollected, as we do today, to ensure that SV rewards are created only once per round.
Therefore, I think we still need joinedAsOfRound to create the initial RewardState for the SV operator. Maybe I’m missing something.
There was a problem hiding this comment.
But won't you have different joinedAsOfRound for every (weighted) SV?
I think this should be part of sv onboarding, not part of the node onboarding.
Either way, next step should be some daml tests. These usually flesh out the details of the different usage scenarios, and help envision much better what the end result would look like. We should have our answers on this once we see those.
There was a problem hiding this comment.
Ok, I wasn't aware that we needed a new onboarding flow for adding new SVs. I thought SV onboarding would only require creating the corresponding on-ledger weight.
How should that flow work?
There was a problem hiding this comment.
Yeah, sorry, that's what I meant by onboarding. Either way, when the SV got added in, that's the minimum round for when they should get rewards. Not when the node was onboarded.
There was a problem hiding this comment.
ok, I will work on that.
| newDsoRules <- create this with svs = newSvs | ||
| return DsoRules_UpdateSvRewardWeightResult with .. | ||
|
|
||
| choice DsoRules_UpdateSvRewardWeight_V2 : DsoRules_UpdateSvRewardWeight_V2Result |
There was a problem hiding this comment.
I think you meant for this to be nonconsuming
(but see my comment below)
| weight = newRewardWeight | ||
| pure DsoRules_UpdateSvRewardWeight_V2Result with svWeightCid | ||
|
|
||
| nonconsuming choice DsoRules_AddSvRewardWeight : DsoRules_AddSvRewardWeightResult |
There was a problem hiding this comment.
Given that the long term direction is for votes to be associated with weights, I think the weights should be part of dsoRules, similarly to how nodes are (or how SVs are today) rather than just standalone contracts between operator&sv.
There was a problem hiding this comment.
But I must admit that I'm not 100% clear on the tradeoffs around this design choice. What's the downside of doing what I proposed? More churn on dsoRules? (which I think is fine, as weights won't change very often)
There was a problem hiding this comment.
It sounds good. I wouldn't expect many weights changes.
In that case, I should get rid of the template SvWeight, right?
There was a problem hiding this comment.
Yes, if we manage it on dsoRules then I don't think we need SvWeight at all.
There was a problem hiding this comment.
ok, I will refactor this model accordingly
Co-authored-by: Itai Segall <itai.segall@digitalasset.com> Signed-off-by: Jose Velasco <jose.velasco@intellecteu.com>
Signed-off-by: Jose Velasco <jose.velasco@intellecteu.com>
Signed-off-by: Jose Velasco <jose.velasco@intellecteu.com>
Signed-off-by: Jose Velasco <jose.velasco@intellecteu.com>
|
@isegall-da |
Summary
This PR adds a Daml draft for the new on-ledger SV weight model. The goal is to represent SV weights fully on-ledger, including SV beneficiary weights.
Main changes:
HostedSvInfoand thehostedSvs/offboardedHostedSvsfields to theDsoRulestemplate to represent SV reward weights separately from legacy SV node/operator state.SvInfofor SV node/operator information, but marksjoinedAsOfRoundandsvRewardWeightas deprecated in favor ofHostedSvInfo.DsoRules_AddHostedSvto add hosted SVs,DsoRules_UpdateHostedSvRewardWeightto update hosted SV weights, andDsoRules_OffboardHostedSvto offboard hosted SVs.DsoRules_AddSvNode, which replacesDsoRules_AddSvfor adding SV nodes. UnlikeDsoRules_AddSv, it does not takenewSvRewardWeightorjoinedAsOfRound, since SV nodes no longer have reward weight attached in the new model.SvRewardBeneficiariestemplate to optionally configure on-ledger beneficiary splits for an SV’s rewards.DsoRules_SetSvRewardBeneficiariesto create, update, or clear beneficiary distributions.DsoRules_MigrateHostedSvs, to migrate an SV operator’s legacy reward weight into hosted SV entries.DsoRules_ReceiveSvRewardCoupon_V2, which creates reward coupons for all SVs operated by an SV operator using hosted SV weights and optional beneficiary splits.DsoRules_UpdateSvRewardWeightandDsoRules_ReceiveSvRewardCoupon, once all SVs have migrated tohostedSvs.Pull Request Checklist
Cluster Testing
/cluster_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./lsu_teston this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n, and mention issues worked on using#nMerge Guidelines