Acp 77 PoS using sdk/validatorManager #2308

arturrez · 2024-11-01T03:30:40Z

Why this should be merged

it adds PoS L1 support to CLI - Native Token for now (ERC20 token - to be added)
it adds transparent proxy to both PoS and PoA validator manager.

How this works

based on PoA/PoS choice it deploys needed smart contracts including transparentProxy.
flows are very similar to PoA except some additional parameters specific to PoS ValidationManagement.
Some params have reasonable defaults to make UX better.

How this was tested

./bin/avalanche blockchain create pos7 --proof-of-stake  --force
./bin/avalanche node local start  --num-nodes 7 --etna-devnet --avalanchego-path ~/src/github.com/ava-labs/avalanchego/build/avalanchego local7
./bin/avalanche blockchain deploy pos7 --cluster local7 --bootstrap-endpoints http://127.0.0.1:9650,http://127.0.0.1:9652,http://127.0.0.1:9654,http://127.0.0.1:9656,http://127.0.0.1:9658 --convert-only
./bin/avalanche node local track local7 pos7
./bin/avalanche contract initPosManager pos7  --cluster local7 --rpc http://127.0.0.1:9650/ext/bc/<BLOCKCHAINID>/rpc
./bin/avalanche blockchain addValidator pos7  --cluster local7 --node-endpoint http://127.0.0.1:9660
./bin/avalanche blockchain addValidator pos7  --cluster local7 --node-endpoint http://127.0.0.1:9662

How is this documented

not yet. Relevant document created by @owenwahlgren https://github.com/ava-labs/etna-devnet-resources/blob/main/guides/deploy-sovereign-pos-l1.md.

It's based on https://github.com/ava-labs/avalanche-cli/tree/acp-77-pos branch created by @owenwahlgren 🙌

…rmanager to make it DRY

add support for proxy Contract for PoA

sdk/validatormanager/validator_manager_pos.go

sukantoraymond · 2024-11-13T17:51:58Z

sdk/validatormanager/root.go

+	if p.MinimumStakeAmount.Cmp(big.NewInt(0)) < 0 {
+		return fmt.Errorf("minimum stake amount cannot be negative")
+	}
+	if p.MaximumStakeAmount.Cmp(big.NewInt(0)) < 0 {
+		return fmt.Errorf("maximum stake amount cannot be negative")
+	}
+	if p.MaximumStakeAmount.Cmp(p.MinimumStakeAmount) < 0 {
+		return fmt.Errorf("maximum stake amount cannot be less than minimum stake amount")
+	}
+	if p.WeightToValueFactor.Cmp(big.NewInt(0)) < 0 {
+		return fmt.Errorf("weight to value factor cannot be negative")
+	}


can they be 0?

this means that WeightToValueFactor can't be 0 and this is what we want

I think for all these, better error is has to be greater than 0, or cannot be empty , compared to cannot be negative

felipemadero · 2024-11-14T02:23:18Z

.github/workflows/e2e-test.yml

@@ -47,7 +48,9 @@ jobs:
          - os: macos-14
            suite: "\\[Public Subnet non SOV\\]"
          - os: ubuntu-latest


why are we excluding ubuntu?

because we relay on avalanchego binary as part of the assets as it's custom build of poc.6 branch.
We can add ubuntu if we replace this prebuilt binary with avalanchego git repo checkout and build as part of the e2e

felipemadero · 2024-11-14T02:24:51Z

cmd/blockchaincmd/add_validator.go

@@ -207,6 +214,17 @@ func promptValidatorBalance(availableBalance uint64) (uint64, error) {
 	return app.Prompt.CaptureValidatorBalance(txt, availableBalance)
 }

+func GetNetworkBalanceForKey(kc *keychain.Keychain, network models.Network) (uint64, error) {


this can go somewhere to utils

name is misleaving because it is a list of keys, that is a keychain

moved to utils. fixed name. good idea

felipemadero · 2024-11-14T02:27:19Z

cmd/blockchaincmd/add_validator.go

+			if err != nil {
+				return err
+			}
+			stakeAmount, err = app.Prompt.CaptureUint64(fmt.Sprintf("Enter the amount of tokens to stake. Available: %d[%s]", avaliableTokens, sc.TokenName))


can we use a capture that disables choosing more than available tokens? (by the way, nit, avaliableTokens should be availableTokens)

cmd/blockchaincmd/create.go

felipemadero · 2024-11-14T03:03:17Z

pkg/prompts/prompts.go

-	includeXChain bool,
-	includeCChain bool,
-	avoidBlockchainName string,
+	avoidPChain bool,


why changing the logic to all avoids?

we double careful this may broke some calls to this func

probably used on key transfer. should check

my bad. fixed

felipemadero · 2024-11-14T03:03:30Z

pkg/prompts/prompts.go

@@ -1063,11 +1109,6 @@ func PromptAddress(
 			if err != nil {
 				return "", err
 			}
-		case XChainFormat:


why removing X-Chain?

my bad. not sure how it happened. checking and putting it back

felipemadero · 2024-11-14T03:11:24Z

pkg/vm/create_evm.go

 	if params.UsePoAValidatorManager {
 		validatormanager.AddPoAValidatorManagerContractToAllocations(params.initialTokenAllocation)
+		validatormanager.AddTransparentProxyContractToAllocations(params.initialTokenAllocation, proxyOwner)
+	} else {
+		validatormanager.AddPoSValidatorManagerContractToAllocations(params.initialTokenAllocation)


can it be this case that it is neither poa neither pos (old stuff)

good catch. replaced with else if

felipemadero · 2024-11-14T03:13:16Z

pkg/vm/evm_prompts.go

@@ -233,6 +238,10 @@ func PromptSubnetEVMGenesisParams(
 		return SubnetEVMGenesisParams{}, "", err
 	}

+	if sc.PoS() || sc.PoA() { // Teleporter bytecode makes genesis too big given the current max size (we include the bytecode for ValidatorManager, a proxy, and proxy admin)
+		params.UseTeleporter = false


this is not enough, you can still use teleporter, the only thing is that it is not going to be included in genesis
can we keep the teleporter flag on but avoid genesis deploy? but instead deploy afterwards

agree. I suggest not fixing it in the scope of this PR and do it as part of the teleporter after blockchain creation deployment PR as we discussed

felipemadero · 2024-11-14T03:13:50Z

pkg/vm/evm_prompts.go

-		params.nativeMinterPrecompileAllowList.EnabledAddresses = []common.Address{
-			common.HexToAddress(validatormanager.ValidatorContractAddress),
-		}
+		params.nativeMinterPrecompileAllowList.AdminAddresses = append(


why admin? is not enough to use enabled?

changed to enabled

…this change is coming from

sukantoraymond · 2024-11-18T17:29:35Z

For deploy, No need to print proxy admin contract address here imo

sukantoraymond · 2024-11-18T17:29:47Z

For addvalidator, Can be 869 AVAX Token, no need for [AVAX Token]

sukantoraymond · 2024-11-18T17:30:00Z

For addvalidator, Dont think we have a minimum duration of 14 days here?

sukantoraymond · 2024-11-18T17:30:33Z

For addvalidator -> lets edit the prompt since now its saying bootstrap validator

* add pos validator removal debug * continue debugging pos validator removal * r4r * lint * fix e2e Error: failure initializing validator removal: min stake duration not passed (tx failed to be submitted) * wait 90sec for min validation period * min stake duration is 100 * update sidecar after bootstrap validator removed * disable e2e for removing PoS bootstrap validator * use 100s * remove pos non bootstrap validator e2e * add validator and remove it * use 60s staking period * go back to 90 and 120s * small refactor * fix duration prompt * use minimum 100s * add prompt check for minimum validator staking timr * fix etna duration input * use etna for name * add debug info * rm debug force validator removal for PoS * no need to updatePchainHeight --------- Signed-off-by: arturrez <[email protected]>

sukantoraymond · 2024-11-18T21:29:12Z

cmd/blockchaincmd/add_validator.go

 	cmd.Flags().BoolVar(&useDefaultStartTime, "default-start-time", false, "(for non sovereign blockchain) use default start time for subnet validator (5 minutes later for fuji & mainnet, 30 seconds later for devnet)")
 	cmd.Flags().StringVar(&startTimeStr, "start-time", "", "(for non sovereign blockchain) UTC start time when this validator starts validating, in 'YYYY-MM-DD HH:MM:SS' format")
 	cmd.Flags().BoolVar(&useDefaultDuration, "default-duration", false, "(for non sovereign blockchain) set duration so as to validate until primary validator ends its period")
 	cmd.Flags().BoolVar(&defaultValidatorParams, "default-validator-params", false, "(for non sovereign blockchain) use default weight/start/duration params for subnet validator")
 	cmd.Flags().StringSliceVar(&subnetAuthKeys, "subnet-auth-keys", nil, "(for non sovereign blockchain) control keys that will be used to authenticate add validator tx")
 	cmd.Flags().StringVar(&outputTxPath, "output-tx-path", "", "(for non sovereign blockchain) file path of the add validator tx")
 	cmd.Flags().BoolVar(&waitForTxAcceptance, "wait-for-tx-acceptance", true, "(for non sovereign blockchain) just issue the add validator tx, without waiting for its acceptance")
+	cmd.Flags().BoolVar(&forcePoS, "pos", false, "(PoS only) force validator initialization as PoS validator")


whats this flag for?

Seems that its for adding to a pos / poa? We can't get from sidecar instead if its poa / pos?

arturrez added 6 commits October 31, 2024 12:51

mv changes frrom pos

6106641

move from pos branch

cfe4556

mocks

4f845ad

sync with latest pos/ refactor PoS to sdk approach

b217756

wip

347406a

fix lint. get rid of example in var names, fix sdk<->pkg for validato…

2c384f4

…rmanager to make it DRY

arturrez self-assigned this Nov 1, 2024

arturrez changed the base branch from main to acp-77 November 1, 2024 03:30

arturrez added the DO NOT MERGE This PR is not meant to be merged in its current state label Nov 1, 2024

arturrez added 21 commits November 1, 2024 12:05

fix err in sdk

7e1add1

PoS default values

301e07f

use sidecar for pos detection with force flag

3f27d47

rename sc validatorManager

b0ef813

use proxy for poa

a7a00d7

bugfixes and improve add validator for pos

68ae8c5

proper errors from validator manager smart contract

f3549d9

rename cmd input

645df62

add PoS e2e

bbd1a05

add support for proxy Contract for PoA

fix lint

6a96c2a

e2e fix arg

7d946f2

fix typo and non needed check

3a60f75

rm unneeded check

81eb204

fix lint

01db0ec

fixes for the flow PoS vs PoA

19529ab

fix pos sidecar case

dad79f0

refactor add validator add cmd params for inputs

c500e42

adjust e2e params

265aff9

use test-defaults for e2e POS

bd7aa1c

fix bug. disable validator removal for now

2eb8dca

fix e2e

9588b12

arturrez added 2 commits November 12, 2024 16:15

force uint64

3ac472f

100s

93277a3

sukantoraymond reviewed Nov 13, 2024

View reviewed changes

sdk/validatormanager/validator_manager_pos.go Show resolved Hide resolved

sukantoraymond reviewed Nov 13, 2024

View reviewed changes

felipemadero reviewed Nov 14, 2024

View reviewed changes

arturrez added 5 commits November 15, 2024 13:08

Merge branch 'acp-77' into acp-77-sdk-pos

4afb263

add validatorManagerSettings to sdk

f712880

provide default params for PoS and move networkBalance to utils

ce0d10d

add prompt verification for token amount input

7e097bb

fix PromptChain. not sure how it was included into this PR and where …

ffed61e

…this change is coming from

sukantoraymond approved these changes Nov 15, 2024

View reviewed changes

arturrez and others added 2 commits November 15, 2024 14:32

address latest feedback

a5d12df

lint

91d53d6

arturrez and others added 3 commits November 18, 2024 12:17

improve user output

49d9874

fix PosManager init

a678fc7

sukantoraymond reviewed Nov 18, 2024

View reviewed changes

arturrez and others added 9 commits November 18, 2024 13:59

use clusterNameFlagValue

94655b9

rm debug

a4329ce

remove incorrect available balance

97d9e37

fix lint

10bc8cc

update avalanchego

f0087bf

update merge

e3a2739

lint

f07fee1

update merge

8d7062a

fix lint

b1acea0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Acp 77 PoS using sdk/validatorManager #2308

Acp 77 PoS using sdk/validatorManager #2308

arturrez commented Nov 1, 2024 •

edited

Loading

sukantoraymond Nov 13, 2024

arturrez Nov 15, 2024 •

edited

Loading

sukantoraymond Nov 18, 2024

felipemadero Nov 14, 2024

arturrez Nov 15, 2024

felipemadero Nov 14, 2024

felipemadero Nov 14, 2024

arturrez Nov 15, 2024

felipemadero Nov 14, 2024

arturrez Nov 15, 2024

felipemadero Nov 14, 2024

felipemadero Nov 14, 2024

felipemadero Nov 14, 2024

arturrez Nov 15, 2024

felipemadero Nov 14, 2024

arturrez Nov 15, 2024 •

edited

Loading

felipemadero Nov 14, 2024

arturrez Nov 15, 2024

felipemadero Nov 14, 2024

arturrez Nov 15, 2024

felipemadero Nov 14, 2024

arturrez Nov 15, 2024

sukantoraymond commented Nov 18, 2024 •

edited

Loading

sukantoraymond commented Nov 18, 2024 •

edited

Loading

sukantoraymond commented Nov 18, 2024 •

edited

Loading

sukantoraymond commented Nov 18, 2024

sukantoraymond Nov 18, 2024

sukantoraymond Nov 18, 2024

Acp 77 PoS using sdk/validatorManager #2308

Are you sure you want to change the base?

Acp 77 PoS using sdk/validatorManager #2308

Conversation

arturrez commented Nov 1, 2024 • edited Loading

Why this should be merged

How this works

How this was tested

How is this documented

Choose a reason for hiding this comment

arturrez Nov 15, 2024 • edited Loading

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

arturrez Nov 15, 2024 • edited Loading

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

sukantoraymond commented Nov 18, 2024 • edited Loading

sukantoraymond commented Nov 18, 2024 • edited Loading

sukantoraymond commented Nov 18, 2024 • edited Loading

sukantoraymond commented Nov 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arturrez commented Nov 1, 2024 •

edited

Loading

arturrez Nov 15, 2024 •

edited

Loading

arturrez Nov 15, 2024 •

edited

Loading

sukantoraymond commented Nov 18, 2024 •

edited

Loading

sukantoraymond commented Nov 18, 2024 •

edited

Loading

sukantoraymond commented Nov 18, 2024 •

edited

Loading