ir: Use genesis role designations for autodeploy #2652

cthulhu-rider · 2023-11-17T16:45:38Z

No description provided.

cthulhu-rider · 2023-11-17T18:47:55Z

tested changes against nspcc-dev/neofs-dev-env#282 and got interested result

NeoFSAlphabet role preset works perfect. Without option:

2023-11-17T18:34:03.469Z	info	deploy/deploy.go:303	initial transfer to the committee successfully done
2023-11-17T18:34:03.469Z	info	deploy/deploy.go:305	initializing NeoFS Alphabet...
2023-11-17T18:34:03.469Z	info	deploy/alphabet.go:58	checking NeoFS Alphabet role of the committee members...
2023-11-17T18:34:03.470Z	info	deploy/alphabet.go:80	not all members of the committee have a NeoFS Alphabet role, designation is needed
2023-11-17T18:34:03.471Z	info	deploy/alphabet.go:98	Notary request designating NeoFS Alphabet role to the committee has been successfully sent, will wait for the outcome	{"main tx": "1cb8a7bdc331ea243273816985803f85d77c03b12e5d8ebd7d52d76adf73821b", "fallback tx": "be5261bbf4842c8fee51dc49fdf53ae2cd90821fa00623a38515211eee11079f", "vub": 173}

and with:

2023-11-17T18:17:27.751Z	info	deploy/deploy.go:303	initial transfer to the committee successfully done
2023-11-17T18:17:27.751Z	info	deploy/deploy.go:305	initializing NeoFS Alphabet...
2023-11-17T18:17:27.751Z	info	deploy/alphabet.go:58	checking NeoFS Alphabet role of the committee members...
2023-11-17T18:17:27.752Z	info	deploy/alphabet.go:76	all committee members have a NeoFS Alphabet role
2023-11-17T18:17:27.752Z	info	deploy/deploy.go:318	NeoFS Alphabet successfully initialized

P2PNotary role works too. Without option:

2023-11-17T18:33:59.450Z	info	deploy/deploy.go:255	NNS contract successfully initialized on the chain	{"address": "2e9a67b9a56562541add67af6056b49b486d67b0"}
2023-11-17T18:33:59.450Z	info	deploy/deploy.go:257	enable Notary service for the committee...
2023-11-17T18:33:59.450Z	info	deploy/notary.go:66	committee is single-acc, no multi-signature needed for Notary role designation
2023-11-17T18:33:59.450Z	info	deploy/notary.go:97	checking Notary role of the committee members...
2023-11-17T18:33:59.450Z	info	deploy/notary.go:119	not all members of the committee have a Notary role, designation is needed
2023-11-17T18:33:59.450Z	info	deploy/notary.go:152	sending new transaction designating Notary role to the local account...
2023-11-17T18:33:59.452Z	info	deploy/notary.go:166	transaction designating Notary role to the local account has been successfully sent, will wait for the outcome	{"tx": "1d3d0a871033b4e38c95e1ee6c85149925f96e64e96ab572cfb3efcc76ab1bf2", "vub": 29}

and with

2023-11-17T18:28:05.211Z	info	deploy/deploy.go:255	NNS contract successfully initialized on the chain	{"address": "2e9a67b9a56562541add67af6056b49b486d67b0"}
2023-11-17T18:28:05.211Z	info	deploy/deploy.go:257	enable Notary service for the committee...
2023-11-17T18:28:05.211Z	info	deploy/notary.go:66	committee is single-acc, no multi-signature needed for Notary role designation
2023-11-17T18:28:05.211Z	info	deploy/notary.go:97	checking Notary role of the committee members...
2023-11-17T18:28:05.212Z	info	deploy/notary.go:115	all committee members have a Notary role
2023-11-17T18:28:05.212Z	info	deploy/deploy.go:273	Notary service successfully enabled for the committee

so, everything seems to be OK.. but initial transfer breaks with P2PNotary role preset. In test env there is only one node that is committee/validators/Alphabet. When set_roles_in_genesis is set,

neofs-node/pkg/morph/deploy/funds.go

Line 59 in 8d9a2bc

    
           func makeInitialTransferToCommittee(ctx context.Context, prm makeInitialGASTransferToCommitteePrm) error {

hangs.

i compared transfer transactions with (when notary request with tx is accepted) and without (hang case) the option. Without:

2023-11-17T18:34:02.464Z	info	deploy/funds.go:121	got current balance of the validator multi-sig account, distributing between the committee...	{"NEO": "100000000", "GAS": "5199999993361180"}
2023-11-17T18:34:02.464Z	info	deploy/funds.go:152	not enough GAS on the committee member's account, need replenishment	{"member": "b3622bf4017bdfe317c58aed5f4c753f206b7db896046fa7d774bbc4bf7f8dc2af9c7b29759df7f3d92052a5b9bc545bcd31c6a7a3463e90c768a6c3e45b1036", "balance": "148374010", "need at least": "15000000000"}
2023-11-17T18:34:02.465Z	info	deploy/funds.go:219	sending new Notary request transferring funds from validator multi-sig account to the committee...
2023-11-17T18:34:02.468Z	info	deploy/funds.go:231	Notary request transferring funds from validator multi-sig account to the committee has been successfully sent, will wait for the outcome	{"main tx": "80c55919db52e69aca03b66ccc5318000663958694b45ff4213c5cf03329339f", "fallback tx": "557e43423ed8a253245837280e0336122b5e4cdd489f19972c731513f692c087", "vub": 172}
2023-11-17T18:34:02.469Z	info	deploy/notary.go:1128	main transaction from the received notary request already has local account's signature, skip	{"tx": "80c55919db52e69aca03b66ccc5318000663958694b45ff4213c5cf03329339f"}
2023-11-17T18:34:03.191Z	info	deploy/notary.go:1096	main transaction of the notary request has already been processed, skip	{"tx": "80c55919db52e69aca03b66ccc5318000663958694b45ff4213c5cf03329339f"}

and with:

2023-11-17T18:28:07.223Z	info	deploy/funds.go:152	not enough GAS on the committee member's account, need replenishment	{"member": "b3622bf4017bdfe317c58aed5f4c753f206b7db896046fa7d774bbc4bf7f8dc2af9c7b29759df7f3d92052a5b9bc545bcd31c6a7a3463e90c768a6c3e45b1036", "balance": "84892290", "need at least": "15000000000"}
2023-11-17T18:28:07.223Z	info	deploy/funds.go:219	sending new Notary request transferring funds from validator multi-sig account to the committee...
2023-11-17T18:28:07.226Z	info	deploy/funds.go:231	Notary request transferring funds from validator multi-sig account to the committee has been successfully sent, will wait for the outcome	{"main tx": "4caaca85ddb5d8046e69057520e11def7c50fc82e96f0811fbc4a239af385959", "fallback tx": "dee0ce208be68a6dd41adf87f2207eac333e52cb74aa897d8b1668d26cb05eec", "vub": 171}
2023-11-17T18:28:07.227Z	info	deploy/notary.go:1128	main transaction from the received notary request already has local account's signature, skip	{"tx": "4caaca85ddb5d8046e69057520e11def7c50fc82e96f0811fbc4a239af385959"}

i checked tx scripts: in both cases they are same. The only observable diff from logs is that no-option case writes two messages main transaction from the received notary request already has local account's signature, skip and main transaction of the notary request has already been processed, skip while with-option case writes only 1st message. Accordingly, when option is unset, Notary request goes to the channel twice (and TX succeeds), and once - when option is set (and TX is lost)

overall, for now i have no idea why process stucks. Need more deep research with fresh head. Help is appreciated

cthulhu-rider · 2023-11-24T08:34:28Z

this is probably related to Notary service non-start, figuring this out with @AnnaShaleva and @roman-khimov ...

cthulhu-rider · 2023-11-26T17:56:36Z

works after fix (revision). Lets wait for it to be released

codecov · 2023-12-02T05:17:25Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (752d8a7) 30.22% compared to head (482e1f7) 30.23%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2652      +/-   ##
==========================================
+ Coverage   30.22%   30.23%   +0.01%     
==========================================
  Files         406      406              
  Lines       30025    30033       +8     
==========================================
+ Hits         9075     9081       +6     
- Misses      20170    20171       +1     
- Partials      780      781       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

AnnaShaleva · 2023-12-04T06:20:55Z

config/example/ir.yaml

+      0: 7 # Maps chain height to number of consensus nodes. Values must be positive up to 'committee' size.
+      1: 5


It can't be 7 and then 5. Please, read the documentation to validators history and committee history option.

AnnaShaleva · 2023-12-04T06:28:03Z

pkg/innerring/config.go

 				return fmt.Errorf("value %d is out of allowable range", num)
+			} else if num > uint32(c.Committee.Len()) {


I don;t quite like these repeating checks. We have https://github.com/nspcc-dev/neo-go/blob/6c0c2a6a98b0799f1e19dfeac8956169f1aa4a7a/pkg/config/protocol_config.go#L80, and if some check is missing there, then create an issue and we'll add more checks.

not this issue's point but interesting, moved to #2666

AnnaShaleva · 2023-12-04T06:30:46Z

config/example/ir.yaml

@@ -77,6 +77,9 @@ morph:
      ping: # Optional settings of pinging mechanism
        interval: 30s # Optional time period between pings. Defaults to 30s. Must not be negative
        timeout: 90s # Optional time period to wait for pong. Defaults to 1m. Must not be negative
+    set_roles_in_genesis: true # Optional flag for designating P2PNotary and NeoFSAlphabet roles to all


AFAIU, genesis-based roles are planned to be a standard for the network. How about reverting the meaning of this setting so that it can be skipped in default config? E.g. s/set_roles_in_genesis/skip_genesis_roles.

propose this in #2643 pls. To me, we aint ready yet to use genesis roles everywhere by default

AnnaShaleva · 2023-12-04T06:32:44Z

pkg/innerring/internal/blockchain/blockchain.go

 	if cfg.ValidatorsHistory != nil {
 		cfgBaseProto.ValidatorsHistory = make(map[uint32]uint32, len(cfg.ValidatorsHistory))
 		for height, num := range cfg.ValidatorsHistory {
 			cfgBaseProto.ValidatorsHistory[height] = num
 		}
+
+		// value checked above in function
+		genesisValidatorsCount = cfg.ValidatorsHistory[0]


Have you considered reusing https://github.com/nspcc-dev/neo-go/blob/6c0c2a6a98b0799f1e19dfeac8956169f1aa4a7a/pkg/config/protocol_config.go#L176 or https://github.com/nspcc-dev/neo-go/blob/6c0c2a6a98b0799f1e19dfeac8956169f1aa4a7a/pkg/config/protocol_config.go#L197?

roman-khimov · 2023-12-06T19:30:16Z

Conflicts though.

pkg/innerring/config.go

Add requirement for heights to be multiples of the committee size. Require value for the zero (genesis) height to be set. Clarify that values must not exceed the committee. Signed-off-by: Leonard Lyubich <[email protected]>

The roles are required to be set for the validators in almost all NeoFS environments. Previously, this required dedicated transaction signed by the committee majority. Starting from release v0.103.0, Neo Go support protocol extension that allows to designate various roles to the various accounts on blockchain genesis (zero height). With this feature, Inner Ring running in local consensus may be at the configuration level. Add `set_roles_in_genesis` value to `morph.consensus` section providing option to Inner Ring to start blockchain with P2PNotary and NeoFSAlphabet roles designated to the genesis block's validators. Closes #2643. Signed-off-by: Leonard Lyubich <[email protected]>

cthulhu-rider force-pushed the feature/2643--ir-genesis-autorole branch 2 times, most recently from dcaad94 to b41ee58 Compare November 17, 2023 16:47

cthulhu-rider changed the title ~~Feature/2643 ir genesis autorole~~ ir: Use genesis role designations for autodeploy Nov 17, 2023

AnnaShaleva mentioned this pull request Nov 24, 2023

Notary service doesn't start its work with GenesisRoles set nspcc-dev/neo-go#3228

Closed

cthulhu-rider mentioned this pull request Nov 26, 2023

Use genesis role designations for autodeploy #2643

Closed

cthulhu-rider force-pushed the feature/2643--ir-genesis-autorole branch from b41ee58 to 1f709c6 Compare December 2, 2023 05:15

cthulhu-rider force-pushed the feature/2643--ir-genesis-autorole branch from 1f709c6 to 0c5928b Compare December 2, 2023 05:21

cthulhu-rider marked this pull request as ready for review December 2, 2023 05:23

cthulhu-rider requested review from roman-khimov and carpawell as code owners December 2, 2023 05:23

AnnaShaleva reviewed Dec 4, 2023

View reviewed changes

cthulhu-rider mentioned this pull request Dec 4, 2023

ir: Use Neo Go protocol config validation in local consensus launch mode #2666

Open

cthulhu-rider force-pushed the feature/2643--ir-genesis-autorole branch 2 times, most recently from 5bb5eb2 to 37a3098 Compare December 4, 2023 11:01

roman-khimov approved these changes Dec 6, 2023

View reviewed changes

cthulhu-rider force-pushed the feature/2643--ir-genesis-autorole branch from 37a3098 to 65513e8 Compare December 6, 2023 19:48

roman-khimov reviewed Dec 6, 2023

View reviewed changes

pkg/innerring/config.go Outdated Show resolved Hide resolved

cthulhu-rider added 2 commits December 7, 2023 08:08

cthulhu-rider force-pushed the feature/2643--ir-genesis-autorole branch from 65513e8 to 482e1f7 Compare December 7, 2023 04:09

roman-khimov merged commit c0d0159 into master Dec 7, 2023
9 of 11 checks passed

roman-khimov deleted the feature/2643--ir-genesis-autorole branch December 7, 2023 10:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ir: Use genesis role designations for autodeploy #2652

ir: Use genesis role designations for autodeploy #2652

cthulhu-rider commented Nov 17, 2023

cthulhu-rider commented Nov 17, 2023 •

edited

Loading

cthulhu-rider commented Nov 24, 2023

cthulhu-rider commented Nov 26, 2023

codecov bot commented Dec 2, 2023 •

edited

Loading

AnnaShaleva Dec 4, 2023

cthulhu-rider Dec 4, 2023

AnnaShaleva Dec 4, 2023

cthulhu-rider Dec 4, 2023

AnnaShaleva Dec 4, 2023

cthulhu-rider Dec 4, 2023

AnnaShaleva Dec 4, 2023

cthulhu-rider Dec 4, 2023

roman-khimov commented Dec 6, 2023

		0: 7 # Maps chain height to number of consensus nodes. Values must be positive up to 'committee' size.
		1: 5

		return fmt.Errorf("value %d is out of allowable range", num)
		} else if num > uint32(c.Committee.Len()) {

ir: Use genesis role designations for autodeploy #2652

ir: Use genesis role designations for autodeploy #2652

Conversation

cthulhu-rider commented Nov 17, 2023

cthulhu-rider commented Nov 17, 2023 • edited Loading

cthulhu-rider commented Nov 24, 2023

cthulhu-rider commented Nov 26, 2023

codecov bot commented Dec 2, 2023 • edited Loading

Codecov Report

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

roman-khimov commented Dec 6, 2023

cthulhu-rider commented Nov 17, 2023 •

edited

Loading

codecov bot commented Dec 2, 2023 •

edited

Loading