Staking V4: Refactor stakingDataProvider to pre-fill auction data #4155
Conversation
mariusmihaic
changed the title
…4-auction-data-preparation
epochStart/dtos.go
Outdated
| type OwnerData struct { | ||
| NumStakedNodes int64 | ||
| NumActiveNodes int64 | ||
| NumAuctionNodes int64 |
There was a problem hiding this comment.
I think NumAuctionNodes can be removed because are duplicated information with the len(AuctionList)
There was a problem hiding this comment.
Right, good suggestion; removed it from here and from stakingDataProvider.
However, I've kept it in auctionListSelector, since there we do a lot of iterations and I don't want to re-calculate len every time.
| flagStakingV4Enable atomic.Flag | ||
| mutStakingData sync.RWMutex | ||
| cache map[string]*ownerStats | ||
| numOfValidatorsInCurrEpoch uint32 |
There was a problem hiding this comment.
numOfValidatorsInCurrEpoch can be moved near stakingV4EnableEpoch in order to group variables of the same type
There was a problem hiding this comment.
Refactored as suggested
| numEligible int | ||
| numStakedNodes int64 | ||
| numActiveNodes int64 | ||
| numAuctionNodes int64 |
There was a problem hiding this comment.
Also here I think you can remove this variable
This adds extra complexity because you have to maintain this
There was a problem hiding this comment.
Right, good suggestion; removed it from here and from stakingDataProvider.
However, I've kept it in auctionListSelector, since there we do a lot of iterations and I don't want to re-calculate len every time.
| @@ -377,7 +458,7 @@ func (sdp *stakingDataProvider) createMapBLSKeyStatus(validatorsInfo state.Shard | |||
| return mapBLSKeyStatus, nil | |||
| } | |||
|
|
|||
| func (sdp *stakingDataProvider) selectKeysToUnStake(sortedKeys map[string][][]byte, numToSelect int64) [][]byte { | |||
| func (sdp *stakingDataProvider) selectKeysToUnStake(sortedKeys map[string][][]byte, numToSelect int64) ([][]byte, int) { | |||
There was a problem hiding this comment.
maybe this function can be renamed
There was a problem hiding this comment.
IMHO, the name is good and reflects its purpose; I would leave it as it is, unless you have a stronger opinion on why it should be renamed
selectedKeys, removedValidators := sdp.selectKeysToUnStake(sortedKeys, numKeysToUnStake)
| @@ -536,3 +536,5 @@ func TestStakingV4_StakeNewNodes(t *testing.T) { | |||
| requireMapContains(t, currNodesConfig.waiting, owner3StakingQueue) | |||
| requireSliceContains(t, currNodesConfig.auction, owner1StakingQueue) | |||
| } | |||
|
|
|||
| // TODO: test unstake with 1 owner -> 1 bls key in auction => numStakedNodes = 0 | |||
There was a problem hiding this comment.
do not forget about this TODO
There was a problem hiding this comment.
Already taken care of in future PRs
|
|
||
| return nil | ||
| } | ||
|
|
||
| // TODO: Move this in elrond-go-core | ||
| func safeSub(a, b uint32) (uint32, error) { |
There was a problem hiding this comment.
we have safeSub for uint64 already in go-core
Moved all logic related to
OwnerDatapre-fill fromauctionListSelectortostakingDataProviderWhat's new
OwnerDatadtoFillValidatorInfonow expects astate.ValidatorInfoHandlerinstead ofblsKeynumOfValidatorsInCurrEpoch, which is the number of validators (waiting + eligible) in the current epoch, needed for auctionListSelector. For each unqualified validator, we decrement this counter.Refactor
unqualifiedOwnersas input param, since this info is provided bystakingDataProviderGetTotalTopUp&GetNumStakedNodesfromstakingDataProvideralong with their testsPrepareStakingDataCallednow expects astate.ShardValidatorsInfoMapHandlerinstead ofmap[uint32][][]byteComputeUnQualifiedNodesetsqualifiedowner's data flag