En 7193/end of epoch sc proc #2219

sasurobert · 2020-08-12T16:42:16Z

end of epoch system smart contract processing. Jailing in the system smart contract the low rating nodes, removing them from staking list, adding users from the waiting list to active.

Start a testnet with MaxNumberOfNodesForStake = number of nodes you run from first epoch.
Stake a set of new nodes, they will enter the waiting list
Turn off some of the initial nodes - only a few of them in order to keep the block production going.
After those nodes rating decreases to minimum, they are jailed and replaced with the ones from the waiting list.

nouserboop · 2020-08-12T16:53:57Z

How to join waiting list please?

iulianpascalau · 2020-08-12T19:25:21Z

epochStart/metachain/systemSCs.go

+
+	var activeStorageUpdate *vmcommon.StorageUpdate
+	for _, storageUpdate := range stakingSCOutput.StorageUpdates {
+		if len(storageUpdate.Offset) == len(jailedValidator.PublicKey) &&


maybe extract the condition in a small bool variable?

how does a big bool variable look like ? 🍡

iulianpascalau · 2020-08-12T19:27:36Z

epochStart/metachain/systemSCs.go

+}
+
+// NewSystemSCProcessor creates the end of epoch system smart contract processor
+func NewSystemSCProcessor(args ArgsNewEpochStartSystemSCProcessing) (*systemSCProcessor, error) {


Missing unit tests.

wanted to have the first review before writing tests.

iulianpascalau · 2020-08-12T19:28:21Z

integrationTests/mock/epochStartSystemSCStub.go

+
+import "github.com/ElrondNetwork/elrond-go/data/state"
+
+type EpochStartSystemSCStub struct {


Missing comment

iulianpascalau · 2020-08-12T19:29:15Z

integrationTests/testProcessorNode.go

@@ -3,6 +3,7 @@ package integrationTests
 import (
 	"encoding/hex"
 	"fmt"
+	"github.com/ElrondNetwork/elrond-go/vm"


unsorted imports.

new goland does not works as expected. done

iulianpascalau · 2020-08-12T19:30:39Z

process/common.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/hex"
 	"fmt"
+	vmcommon "github.com/ElrondNetwork/elrond-vm-common"


unsorted imports

iulianpascalau · 2020-08-12T19:31:44Z

process/common.go

@@ -652,3 +653,42 @@ func IsAllowedToSaveUnderKey(key []byte) bool {
 	trimmedKey := key[:prefixLen]
 	return !bytes.Equal(trimmedKey, []byte(core.ElrondProtectedKeyPrefix))
 }
+
+// SortVMOutputInsideData returns the output accounts as a sorted list
+func SortVMOutputInsideData(vmOutput *vmcommon.VMOutput) []*vmcommon.OutputAccount {


unit tests?

in the smart contract process there are tests directly using this function

iulianpascalau · 2020-08-12T19:32:06Z

process/mock/epochStartSystemSCStub.go

+
+import "github.com/ElrondNetwork/elrond-go/data/state"
+
+type EpochStartSystemSCStub struct {


missing comment

iulianpascalau · 2020-08-13T04:44:42Z

process/scToProtocol/stakingToPeer.go

@@ -227,7 +227,7 @@ func (stp *stakingToPeer) updatePeerState(
 			account.SetTempRating(stp.unJailRating)
 		}

-		if !isValidator && account.GetUnStakedEpoch() == core.DefaultUnstakedEpoch {
+		if !isValidator && account.GetUnStakedEpoch() == core.DefaultUnstakedEpoch && stakingData.Staked == true {


Maybe extract in a bool var?

iulianpascalau · 2020-08-13T04:45:24Z

process/transaction/shardProcess_test.go

@@ -5,6 +5,7 @@ import (
 	"crypto/rand"
 	"encoding/hex"
 	"errors"
+	"github.com/ElrondNetwork/elrond-go/vm"


unsorted imports

iulianpascalau · 2020-08-13T04:46:52Z

vm/systemSmartContracts/staking.go

@@ -999,6 +1008,49 @@ func (r *stakingSC) createWaitingListKey(blsKey []byte) []byte {
 	return []byte(waitingElementPrefix + string(blsKey))
 }

+func (r *stakingSC) switchJailedWithWaiting(args *vmcommon.ContractCallInput) vmcommon.ReturnCode {


…to EN-7193/end-of-epoch-sc-proc

SebastianMarian · 2020-08-13T08:19:07Z

epochStart/metachain/systemSCs.go

+
+// NewSystemSCProcessor creates the end of epoch system smart contract processor
+func NewSystemSCProcessor(args ArgsNewEpochStartSystemSCProcessing) (*systemSCProcessor, error) {
+	if check.IfNilReflect(args.SystemVM) {


IfNilReflect ?

yep. systemVM interface does not have isInterfacenil

SebastianMarian · 2020-08-13T08:21:52Z

epochStart/metachain/systemSCs.go

+func (s *systemSCProcessor) swapJailedWithWaiting(validatorInfos map[uint32][]*state.ValidatorInfo) error {
+	jailedValidators := s.getSortedJailedNodes(validatorInfos)
+	for _, jailedValidator := range jailedValidators {
+


Remove empty line

SebastianMarian · 2020-08-13T08:31:19Z

epochStart/metachain/systemSCs.go

+func (s *systemSCProcessor) processSCOutputAccounts(
+	vmOutput *vmcommon.VMOutput,
+) error {
+


Remove empty line

SebastianMarian · 2020-08-13T08:40:17Z

process/block/metablock.go

 	if err != nil {
 		return err
 	}

-	err = mp.scToProtocol.UpdateProtocol(body, header.Nonce)


So, this is not needed anymore?

in case of epoch start block is not needed.

there are no smart contract results in the epoch start block - thus this function did nothing.

SebastianMarian · 2020-08-13T08:44:13Z

process/common.go

+
+	outPutAccounts := make([]*vmcommon.OutputAccount, len(vmOutput.OutputAccounts))
+	i := 0
+	for _, outAcc := range vmOutput.OutputAccounts {


Why not: index, outAcc and remove line 667 and 670?

output accounts is a map.

SebastianMarian · 2020-08-13T08:47:05Z

process/common.go

+func GetSortedStorageUpdates(account *vmcommon.OutputAccount) []*vmcommon.StorageUpdate {
+	storageUpdates := make([]*vmcommon.StorageUpdate, len(account.StorageUpdates))
+	i := 0
+	for _, update := range account.StorageUpdates {


Why not: index, update and remove line 683 and 686?

storageUpdates is a map

SebastianMarian · 2020-08-13T08:50:19Z

process/scToProtocol/stakingToPeer.go

@@ -227,7 +227,7 @@ func (stp *stakingToPeer) updatePeerState(
 			account.SetTempRating(stp.unJailRating)
 		}

-		if !isValidator && account.GetUnStakedEpoch() == core.DefaultUnstakedEpoch {
+		if !isValidator && account.GetUnStakedEpoch() == core.DefaultUnstakedEpoch && stakingData.Staked == true {


&& stakingData.Staked is enough

SebastianMarian · 2020-08-13T09:03:49Z

vm/systemSmartContracts/staking.go

+	}
+
+	r.removeFromStakedNodes()
+	registrationData.Staked = false


registration data will contain the information related to the waiting or to the jailed node?

it already has.

SebastianMarian · 2020-08-13T09:06:44Z

process/scToProtocol/stakingToPeer.go

@@ -227,7 +227,8 @@ func (stp *stakingToPeer) updatePeerState(
 			account.SetTempRating(stp.unJailRating)
 		}

-		if !isValidator && account.GetUnStakedEpoch() == core.DefaultUnstakedEpoch && stakingData.Staked == true {
+		isNewValidator := !isValidator && account.GetUnStakedEpoch() == core.DefaultUnstakedEpoch && stakingData.Staked == true


... && stakingData.Staked is enough

iulianpascalau

Unit tests should be added soon as to not forget about them.

…to EN-7193/end-of-epoch-sc-proc

LucianMincu · 2020-08-15T13:23:24Z

How to join waiting list please?

@nouserboop Make sure you follow our validator channels. In the following weeks we will deploy the functions to the mainnet and than it will be just a matter of enable-ing the functions for everyone so you'll need to be ready if you want to make sure you'll get one seat in front of everyone. 👍

iulianpascalau · 2020-08-15T13:32:21Z

epochStart/metachain/systemSCs_test.go

+	"github.com/ElrondNetwork/elrond-go/hashing"
+	"github.com/ElrondNetwork/elrond-go/hashing/sha256"
+	"github.com/ElrondNetwork/elrond-go/marshal"
+	economics2 "github.com/ElrondNetwork/elrond-go/process/economics"


LucianMincu

System tests passed.

sasurobert added 2 commits August 12, 2020 17:47

swap jailed nodes with waiting nodes

a65d245

fixing tests and cyclic import

7c5e725

iulianpascalau self-requested a review August 12, 2020 18:37

iulianpascalau requested changes Aug 13, 2020

View reviewed changes

SebastianMarian self-requested a review August 13, 2020 06:49

sasurobert added 3 commits August 13, 2020 11:47

Merge branch 'development' into EN-7193/end-of-epoch-sc-proc

ad456ab

fix after review

56b4dfd

Merge remote-tracking branch 'origin/EN-7193/end-of-epoch-sc-proc' in…

b44c702

…to EN-7193/end-of-epoch-sc-proc

SebastianMarian reviewed Aug 13, 2020

View reviewed changes

fix after review

dada226

SebastianMarian previously approved these changes Aug 13, 2020

View reviewed changes

Merge branch 'development' into EN-7193/end-of-epoch-sc-proc

b85529a

iulianpascalau previously approved these changes Aug 13, 2020

View reviewed changes

sasurobert added 2 commits August 14, 2020 10:18

unit tests added

166c5f4

Merge remote-tracking branch 'origin/EN-7193/end-of-epoch-sc-proc' in…

b75a581

…to EN-7193/end-of-epoch-sc-proc

sasurobert dismissed stale reviews from iulianpascalau and SebastianMarian via b75a581 August 14, 2020 07:19

Merge branch 'development' into EN-7193/end-of-epoch-sc-proc

3eddb4c

SebastianMarian approved these changes Aug 14, 2020

View reviewed changes

LucianMincu added 2 commits August 14, 2020 21:08

Merge branch 'development' into EN-7193/end-of-epoch-sc-proc

f67ea37

Merge branch 'development' into EN-7193/end-of-epoch-sc-proc

c77e4e1

iulianpascalau approved these changes Aug 15, 2020

View reviewed changes

Merge branch 'development' into EN-7193/end-of-epoch-sc-proc

414bb6c

LucianMincu approved these changes Aug 15, 2020

View reviewed changes

LucianMincu merged commit 98c7890 into development Aug 15, 2020

LucianMincu deleted the EN-7193/end-of-epoch-sc-proc branch August 15, 2020 20:20


		import "github.com/ElrondNetwork/elrond-go/data/state"

		type EpochStartSystemSCStub struct {

En 7193/end of epoch sc proc #2219

En 7193/end of epoch sc proc #2219

Conversation

sasurobert commented Aug 12, 2020

nouserboop commented Aug 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iulianpascalau left a comment

Choose a reason for hiding this comment

LucianMincu commented Aug 15, 2020 • edited

Choose a reason for hiding this comment

LucianMincu left a comment

Choose a reason for hiding this comment

LucianMincu commented Aug 15, 2020 •

edited