Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

En 12302 cosigned set guardian #4119

Merged
merged 5 commits into from
Jun 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cmd/node/config/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
# than this value, then the validators rating decrease should stop so they won't get jailed
MaxConsecutiveRoundsOfRatingDecrease = 600

# SetGuardianEpochsDelay represents the delay in epochs between the execution time of the SetGuardian transaction and
# the activation of the configured guardian.
# Make sure that this is greater than the unbonding period!
SetGuardianEpochsDelay = 20

[Versions]
DefaultVersion = "default"
VersionsByEpochs = [
Expand Down Expand Up @@ -66,6 +71,7 @@
# it is a good idea to increase the maximum number of opened files allowed by the operating system
FullArchiveNumActivePersisters = 10


Copy link
Contributor

Choose a reason for hiding this comment

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

remove extra line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done on #4159

[MiniBlocksStorage]
[MiniBlocksStorage.Cache]
Name = "MiniBlocksStorage"
Expand Down
1 change: 1 addition & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ type GeneralSettingsConfig struct {
MinTransactionVersion uint32
GenesisString string
GenesisMaxNumberOfShards uint32
SetGuardianEpochsDelay uint32
}

// FacadeConfig will hold different configuration option that will be passed to the main ElrondFacade
Expand Down
3 changes: 2 additions & 1 deletion factory/bootstrapComponents.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ func (bcf *bootstrapComponentsFactory) Create() (*bootstrapComponents, error) {

dataSyncerFactory := bootstrap.NewScheduledDataSyncerFactory()

guardedAccountHandler, err := guardian.NewGuardedAccount(bcf.coreComponents.InternalMarshalizer(), bcf.coreComponents.EpochNotifier())
setGuardianEpochsDelay := bcf.config.GeneralSettings.SetGuardianEpochsDelay
guardedAccountHandler, err := guardian.NewGuardedAccount(bcf.coreComponents.InternalMarshalizer(), bcf.coreComponents.EpochNotifier(), setGuardianEpochsDelay)
if err != nil {
return nil, err
}
Expand Down
1 change: 1 addition & 0 deletions factory/coreComponents_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ func getCoreArgs() factory.CoreComponentsFactoryArgs {
ChainID: "undefined",
MinTransactionVersion: 1,
GenesisMaxNumberOfShards: 3,
SetGuardianEpochsDelay: 20,
},
Marshalizer: config.MarshalizerConfig{
Type: testMarshalizer,
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ require (
github.com/ElrondNetwork/elrond-go-core v1.1.15-0.20220517131228-41edc685421f
github.com/ElrondNetwork/elrond-go-crypto v1.0.1
github.com/ElrondNetwork/elrond-go-logger v1.0.6
github.com/ElrondNetwork/elrond-vm-common v1.3.3-0.20220517132451-65bc142a2e5f
github.com/ElrondNetwork/elrond-vm-common v1.3.3-0.20220523152821-4a1f295414c2
github.com/ElrondNetwork/notifier-go v1.0.3
github.com/beevik/ntp v0.3.0
github.com/btcsuite/btcd v0.22.0-beta
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ github.com/ElrondNetwork/elrond-go-logger v1.0.6/go.mod h1:cBfgx0ST/CJx8jrxJSC5a
github.com/ElrondNetwork/elrond-vm-common v1.1.0/go.mod h1:w3i6f8uiuRkE68Ie/gebRcLgTuHqvruJSYrFyZWuLrE=
github.com/ElrondNetwork/elrond-vm-common v1.2.9/go.mod h1:B/Y8WiqHyDd7xsjNYsaYbVMp1jQgQ+z4jTJkFvj/EWI=
github.com/ElrondNetwork/elrond-vm-common v1.3.3-0.20220216133549-5b9bf359281c/go.mod h1:XQoxE2MmrhVpyBMghK3fWvnRAw/iirc0KPtbKAOEPBM=
github.com/ElrondNetwork/elrond-vm-common v1.3.3-0.20220517132451-65bc142a2e5f h1:kwSsEEkQ8Y4LEsS0qAfGh3oTVV5hPZ5nOhIzDuhfHUg=
github.com/ElrondNetwork/elrond-vm-common v1.3.3-0.20220517132451-65bc142a2e5f/go.mod h1:MytqqfkrTb1CLIMRVY6ayAwLe6P8Rleey0tSvSQcljA=
github.com/ElrondNetwork/elrond-vm-common v1.3.3-0.20220523152821-4a1f295414c2 h1:Ocemk7ZBNqysUHCpcfTSo8jCmmLq4vKDjmDIwRMJSM4=
github.com/ElrondNetwork/elrond-vm-common v1.3.3-0.20220523152821-4a1f295414c2/go.mod h1:MytqqfkrTb1CLIMRVY6ayAwLe6P8Rleey0tSvSQcljA=
github.com/ElrondNetwork/notifier-go v1.0.3 h1:LhecyXqKuc/Q4NtIOlb9rw4hfMSj6usmxvYQWvb7Pn4=
github.com/ElrondNetwork/notifier-go v1.0.3/go.mod h1:GOv7j2o90e/GVmmBddWwyfJZ/JW4V4pvyXIpukd6FfY=
github.com/ElrondNetwork/protobuf v1.3.2 h1:qoCSYiO+8GtXBEZWEjw0WPcZfM3g7QuuJrwpN+y6Mvg=
Expand Down
4 changes: 2 additions & 2 deletions process/dataValidators/txValidator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,6 @@ func getDefaultInterceptedTx() *mock.InterceptedTxHandlerStub {
func createDummyFrozenAccount() state.UserAccountHandler {
acc := state.NewEmptyUserAccount()
metadata := &vmcommon.CodeMetadata{Frozen: true}
acc.SetCodeMetadata( metadata.ToBytes())
acc.SetCodeMetadata(metadata.ToBytes())
return acc
}
}
7 changes: 5 additions & 2 deletions process/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -1089,8 +1089,8 @@ var ErrNilGuardianSigVerifier = errors.New("nil guardian signature verifier")
// ErrAccountHasNoGuardianSet signals that the account has no guardians set
var ErrAccountHasNoGuardianSet = errors.New("account has no guardian set")

// ErrActiveHasNoActiveGuardian signals that the account has no active guardian
var ErrActiveHasNoActiveGuardian = errors.New("account has no active guardian")
// ErrAccountHasNoActiveGuardian signals that the account has no active guardian
var ErrAccountHasNoActiveGuardian = errors.New("account has no active guardian")

// ErrNilGuardedAccountHandler signals that a nil guarded account handler was provided
var ErrNilGuardedAccountHandler = errors.New("nil guarded account handler")
Expand All @@ -1100,3 +1100,6 @@ var ErrOperationNotPermitted = errors.New("operation in account not permitted")

// ErrTransactionAndAccountGuardianMismatch signals a mismatch between the guardian on the account and the one on the transaction
var ErrTransactionAndAccountGuardianMismatch = errors.New("mismatch between transaction guardian and configured account guardian")

// ErrInvalidSetGuardianEpochsDelay signals an invalid configuration for the epochs delay
var ErrInvalidSetGuardianEpochsDelay = errors.New("incorrect setting for set guardian epochs delay")
2 changes: 1 addition & 1 deletion process/guardian/disabled/disabledGuardedAccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func (dga *disabledGuardedAccount) GetActiveGuardian(_ vmcommon.UserAccountHandl
}

// SetGuardian returns nil as this is a disabled implementation
func (dga *disabledGuardedAccount) SetGuardian(_ vmcommon.UserAccountHandler, _ []byte) error {
func (dga *disabledGuardedAccount) SetGuardian(_ vmcommon.UserAccountHandler, _ []byte, _ []byte) error {
return nil
}

Expand Down
86 changes: 67 additions & 19 deletions process/guardian/guardedAccount.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package guardian

import (
"bytes"
"fmt"
"sync"

Expand All @@ -15,29 +16,34 @@ import (

var guardianKey = []byte(core.ElrondProtectedKeyPrefix + core.GuardiansKeyIdentifier)

const epochsForActivation = 10 // TODO: take from config

type guardedAccount struct {
marshaller marshal.Marshalizer
epochNotifier vmcommon.EpochNotifier
mutEpoch sync.RWMutex
currentEpoch uint32
guardianActivationEpochs uint32
marshaller marshal.Marshalizer
epochNotifier vmcommon.EpochNotifier
mutEpoch sync.RWMutex
guardianActivationEpochsDelay uint32
currentEpoch uint32
}

// NewGuardedAccount creates a new guarded account
func NewGuardedAccount(marshaller marshal.Marshalizer, epochNotifier vmcommon.EpochNotifier) (*guardedAccount, error) {
func NewGuardedAccount(
marshaller marshal.Marshalizer,
epochNotifier vmcommon.EpochNotifier,
setGuardianEpochsDelay uint32,
) (*guardedAccount, error) {
if check.IfNil(marshaller) {
return nil, process.ErrNilMarshalizer
}
if check.IfNil(epochNotifier) {
return nil, process.ErrNilEpochNotifier
}
if setGuardianEpochsDelay == 0 {
return nil, process.ErrInvalidSetGuardianEpochsDelay
}

agc := &guardedAccount{
marshaller: marshaller,
epochNotifier: epochNotifier,
guardianActivationEpochs: epochsForActivation,
marshaller: marshaller,
epochNotifier: epochNotifier,
guardianActivationEpochsDelay: setGuardianEpochsDelay,
}

epochNotifier.RegisterNotifyHandler(agc)
Expand Down Expand Up @@ -69,17 +75,21 @@ func (agc *guardedAccount) GetActiveGuardian(uah vmcommon.UserAccountHandler) ([
}

// SetGuardian sets a guardian for an account
func (agc *guardedAccount) SetGuardian(uah vmcommon.UserAccountHandler, guardianAddress []byte) error {
guardian := &guardians.Guardian{
Address: guardianAddress,
ActivationEpoch: agc.currentEpoch + agc.guardianActivationEpochs,
}

func (agc *guardedAccount) SetGuardian(uah vmcommon.UserAccountHandler, guardianAddress []byte, txGuardianAddress []byte) error {
stateUserAccount, ok := uah.(state.UserAccountHandler)
if !ok {
return process.ErrWrongTypeAssertion
}

if len(txGuardianAddress) > 0 {
return agc.instantSetGuardian(stateUserAccount, guardianAddress, txGuardianAddress)
}

guardian := &guardians.Guardian{
Address: guardianAddress,
ActivationEpoch: agc.currentEpoch + agc.guardianActivationEpochsDelay,
}

return agc.setAccountGuardian(stateUserAccount, guardian)
}

Expand All @@ -88,6 +98,7 @@ func (agc *guardedAccount) setAccountGuardian(uah state.UserAccountHandler, guar
if err != nil {
return err
}

newGuardians, err := agc.updateGuardians(guardian, configuredGuardians)
if err != nil {
return err
Expand All @@ -101,6 +112,43 @@ func (agc *guardedAccount) setAccountGuardian(uah state.UserAccountHandler, guar
return agc.saveAccountGuardians(accHandler, newGuardians)
}

func (agc *guardedAccount) instantSetGuardian(
uah state.UserAccountHandler,
guardianAddress []byte,
txGuardianAddress []byte,
) error {
accountGuardians, err := agc.getConfiguredGuardians(uah)
if err != nil {
return err
}

activeGuardian, err := agc.getActiveGuardian(accountGuardians)
if err != nil {
return err
}

if !bytes.Equal(activeGuardian.Address, txGuardianAddress) {
return process.ErrTransactionAndAccountGuardianMismatch
}

// immediately set the new guardian
guardian := &guardians.Guardian{
Address: guardianAddress,
ActivationEpoch: agc.currentEpoch,
}

accountGuardians.Slice = []*guardians.Guardian{guardian}
accHandler, ok := uah.(vmcommon.UserAccountHandler)
if !ok {
return process.ErrWrongTypeAssertion
}

return agc.saveAccountGuardians(accHandler, accountGuardians)
}

// TODO: add constraints on not co-signed txs on interceptor, for setGuardian
Copy link
Contributor

Choose a reason for hiding this comment

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

task for TODO?

Also, question: is there a check as to not send value on a setGuardian transaction call? (in builtin function implementation)

Copy link
Contributor Author

@AdoAdoAdo AdoAdoAdo May 27, 2022

Choose a reason for hiding this comment

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

Created a task: [EN-12365] for the TODO.

the check is for all related builtin functions: SetGuardian, FreezeAccount & UnfreezeAccount

The check is already done on checkBaseAccountFreezerArgs for all related builtin functions

        if !isZero(vmInput.CallValue) {
		return ErrBuiltInFunctionCalledWithValue
	}

// 1. Gas price cannot exceed a preconfigured limit
// 2. If there is already one guardian pending, do not allow setting another one
func (agc *guardedAccount) updateGuardians(newGuardian *guardians.Guardian, accountGuardians *guardians.Guardians) (*guardians.Guardians, error) {
numSetGuardians := len(accountGuardians.Slice)

Expand All @@ -115,7 +163,7 @@ func (agc *guardedAccount) updateGuardians(newGuardian *guardians.Guardian, acco
return nil, fmt.Errorf("%w in updateGuardians, with %d configured guardians", err, numSetGuardians)
}

if activeGuardian.Equal(newGuardian) {
if bytes.Equal(activeGuardian.Address, newGuardian.Address) {
accountGuardians.Slice = []*guardians.Guardian{activeGuardian}
} else {
accountGuardians.Slice = []*guardians.Guardian{activeGuardian, newGuardian}
Expand Down Expand Up @@ -175,7 +223,7 @@ func (agc *guardedAccount) getActiveGuardian(gs *guardians.Guardians) (*guardian
}

if selectedGuardian == nil {
return nil, process.ErrActiveHasNoActiveGuardian
return nil, process.ErrAccountHasNoActiveGuardian
}

return selectedGuardian, nil
Expand Down
Loading