Stop supporting non-notary deployments #324

cthulhu-rider · 2023-02-10T12:54:30Z

closes Drop notary-less code and data from all contracts except mainnet one #303

clean storage
remove non-notary node from methods
distribute GAS on Alphabet contract update
unit tests

alphabet/alphabet_contract.go

audit/audit_contract.go

neofsid/neofsid_contract.go

netmap/netmap_contract.go

subnet/subnet_contract.go

reputation/reputation_contract.go

roman-khimov

Adding these now, will check tests later.

alphabet/alphabet_contract.go

balance/balance_contract.go

roman-khimov

Have you tried it on the mainnet data?

roman-khimov · 2023-02-27T15:13:45Z

tests/migration/model.go

+
+// UpdateModel describes storage migration model of some Neo contract. Zero update model
+// describes empty post-update contract storage.
+type UpdateModel struct {


This model describes change set in terms of KV pairs, which is fine when:

we delete some key or a set of keys with some prefix and want to ensure it's gone

we specifically add some key or a set of keys and want to ensure they're present

we want to make sure no excessive elements are added

But it's suboptimal when we're dealing with changes. Consider our netmap 0.16.0 migration problem (df2bad0), if we have this omission in the contract we're quite likely to have the same omission in the test, but calling contract APIs could quickly reveal the problem, so I'd say we should describe these in terms of API calls (test invocations) that should have the same (or intentionally different if needed) result before the migration and after the migration.

IMO these are two different tests:

migration of storage items from one format to another

ability of new contract executable to correctly decode new format

2nd won't help us if there is no corresponding values in the contract at all. But I agree that such API extra check will help us to feel more comfortable.

Well, if you're to rely on 2 then you'll catch problems with 1 as well. Contract APIs is what gives access to this data, outside users don't care much about KVs behind these APIs. So I'd say API-bound model is stronger. It may be nontrivial to test in some cases, but it's stronger in general.

tests/migration/storage.go

roman-khimov · 2023-02-27T15:30:43Z

tests/migration/storage.go

+	// 5.
+	contractUpd := newContractUpdater(tb, bLocal, opts.SourceCodeDir, localContract)
+
+	for _, updFail := range opts.UpdateFailures {


These better be independent t.Run() and maybe better in the test itself.

tests/migration/storage.go

Caught while testing nspcc-dev/neofs-contract#324 with removed `innerRingList` method. Implementation did not comply with the requirements https://pkg.go.dev/github.com/nspcc-dev/neofs-contract@v0.16.0/netmap#InnerRingList

Upgrade to the latest revisions of `neo-go`, `pkg/interop` and `testify` modules. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

Make update routine to panic if `notaryDisabled` flag is set. Remove all notary-less artifacts from the contract (storage and methods) on update. Prevent from updating during pending votes. Make GAS transfers on Alphabet contract's update. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

Add `tests/dump` package with `Creator` and `Reader` types which provide I/O operations of contract states (including storages). Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

Add `cmd/dump` command which accepts Neo RPC endpoint of the NeoFS network and dumps NeoFS contracts into `testdata` directory. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

In previous implementation `common.CheckVersion` allowed contract to be updated to earlier versions. This could be a problem since currently forward-only updates are supported. Make `common.CheckVersion` to panic if original contract version is less than `common.Version` one. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

`defaultRegisterPrice` constant is used in `_deploy` function of the NNS contract. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

See #2612, nspcc-dev/neofs-contract#324. Refs #830. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

After fb13ef9, it is now possible not to create an additional notary request but to sign the original one. See also nspcc-dev/neofs-contract#324 that includes SN in a netmap with `AddPeer` netmap call only (without the formly required `AddPeerIR` call). Closes #830. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

cthulhu-rider requested a review from AnnaShaleva February 10, 2023 12:55

roman-khimov reviewed Feb 10, 2023

View reviewed changes

subnet/subnet_contract.go Show resolved Hide resolved

reputation/reputation_contract.go Show resolved Hide resolved

cthulhu-rider force-pushed the 303-drop-non-notary branch from 0653d82 to f47058c Compare February 13, 2023 18:10

cthulhu-rider force-pushed the 303-drop-non-notary branch from 8ebf2db to 056e5bd Compare February 21, 2023 10:43

roman-khimov reviewed Feb 22, 2023

View reviewed changes

cthulhu-rider force-pushed the 303-drop-non-notary branch from b076ebf to 46c46be Compare February 22, 2023 16:51

roman-khimov reviewed Feb 27, 2023

View reviewed changes

cthulhu-rider force-pushed the 303-drop-non-notary branch 5 times, most recently from a32980a to fca6c3a Compare March 3, 2023 15:56

cthulhu-rider mentioned this pull request Mar 6, 2023

Drop notary-less code cthulhu-rider/neofs-contract#1

Closed

cthulhu-rider force-pushed the 303-drop-non-notary branch from 26f0d0d to 4145870 Compare March 16, 2023 13:18

cthulhu-rider marked this pull request as ready for review March 16, 2023 13:21

cthulhu-rider requested a review from roman-khimov March 16, 2023 13:29

cthulhu-rider mentioned this pull request Mar 16, 2023

IR: Fix incorrect way of IR listing with notary Sidechain nspcc-dev/neofs-node#2283

Merged

cthulhu-rider force-pushed the 303-drop-non-notary branch 3 times, most recently from 5629bfc to 04af510 Compare March 17, 2023 13:18

go.mod: Upgrade NeoGo modules

a54d304

Upgrade to the latest revisions of `neo-go`, `pkg/interop` and `testify` modules. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

cthulhu-rider force-pushed the 303-drop-non-notary branch from 04af510 to 465a6ad Compare March 17, 2023 13:21

cthulhu-rider added 6 commits March 17, 2023 17:29

Provide functionality to dump NeoFS contracts locally

00959ef

Add `tests/dump` package with `Creator` and `Reader` types which provide I/O operations of contract states (including storages). Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

Provide command to dump NeoFS contracts from the remote network

13246b6

Add `cmd/dump` command which accepts Neo RPC endpoint of the NeoFS network and dumps NeoFS contracts into `testdata` directory. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

Provide test framework for contracts' migration

808710c

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

Test migration of the contracts to notary mode

bd7297a

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

cthulhu-rider added 3 commits March 17, 2023 17:29

Dump current data of the contract from NeoFS TestNet and MainNet

b623625

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

common: Bump contract versions to v0.17.0

6971785

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

Fix unused linter's remark

78ecc40

`defaultRegisterPrice` constant is used in `_deploy` function of the NNS contract. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

cthulhu-rider force-pushed the 303-drop-non-notary branch from 465a6ad to 78ecc40 Compare March 17, 2023 13:30

roman-khimov approved these changes Mar 17, 2023

View reviewed changes

roman-khimov merged commit 6448e23 into nspcc-dev:master Mar 17, 2023

This was referenced Mar 17, 2023

Improve contract update and migration #301

Closed

Add contract update tests #281

Closed

carpawell added a commit to nspcc-dev/neofs-node that referenced this pull request Jan 12, 2024

ir: Do not create new notary requests for SN netmap entrances

4eee5b0

See #2612, nspcc-dev/neofs-contract#324. Refs #830. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

carpawell mentioned this pull request Jan 12, 2024

ir: Do not create new notary requests for SN netmap entrances nspcc-dev/neofs-node#2717

Merged

carpawell added a commit to nspcc-dev/neofs-node that referenced this pull request Jan 12, 2024

ir: Do not create new notary requests for SN netmap entrances

5d26195

See #2612, nspcc-dev/neofs-contract#324. Refs #830. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>

roman-khimov mentioned this pull request Feb 9, 2024

Load and export storage to json neo-project/neo#3127

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stop supporting non-notary deployments #324

Stop supporting non-notary deployments #324

cthulhu-rider commented Feb 10, 2023 •

edited

Loading

roman-khimov left a comment

roman-khimov left a comment

roman-khimov Feb 27, 2023

cthulhu-rider Feb 28, 2023

roman-khimov Feb 28, 2023

roman-khimov Feb 27, 2023

Stop supporting non-notary deployments #324

Stop supporting non-notary deployments #324

Conversation

cthulhu-rider commented Feb 10, 2023 • edited Loading

roman-khimov left a comment

Choose a reason for hiding this comment

roman-khimov left a comment

Choose a reason for hiding this comment

roman-khimov Feb 27, 2023

Choose a reason for hiding this comment

cthulhu-rider Feb 28, 2023

Choose a reason for hiding this comment

roman-khimov Feb 28, 2023

Choose a reason for hiding this comment

roman-khimov Feb 27, 2023

Choose a reason for hiding this comment

cthulhu-rider commented Feb 10, 2023 •

edited

Loading