cli: db download-contract command #3960

ixje · 2025-07-21T13:31:24Z

Problem

We (coz) have used the feature (in neo-express) for a couple of years now and have learned that

multi-node support really isn't worth supporting
not knowing contract child dependencies so far hasn't been an issue (point 3. of original issue)
similarly point 4. of the original issue is theoretically a "problem" but in practice not yet encountered. So again can be ignored for the time being.
in our experience the ability to only overwrite the contract storage or only the contract state (options in neo-express) is hardly used (more on this below)

Solution

The still relevant parts are addressed with this PR, being the basic functionality of fetching a contract state from a remote chain and storing it locally. From practice this was always done when the node was not running, so I implemented it as a subcommand of neo-go db.

The most relevant discussion imo is how to handle the persisting to storage, specifically around contract IDs. In neo-express I implemented it as follows

if the remote contract hash does not exist, persist 1:1
if it does exist, overwrite the state if --force is specified (but keep the local contract ID)

This PR is slightly different where for 1. we do an additional check to see if the the remote contract ID is not already in use locally.

open things to address

after running the db download-contract command, the first time the node is started the new contract cannot be found (via the getcontractstate RPC method). Only after a shutdown and starting again it can be found. It's unclear to me why
the 2 TODOs in the code. I'm sure there is an opinion from your side how to expose the required data. I didn't want to guess, so took the easiest approach for me to start the discussion.

roman-khimov · 2025-07-21T13:38:51Z

Related: https://github.com/nspcc-dev/neofs-contract/blob/master/cmd/dump/main.go, https://pkg.go.dev/github.com/nspcc-dev/neofs-contract/tests/dump.

codecov · 2025-07-21T14:00:22Z

Codecov Report

Attention: Patch coverage is 14.89362% with 160 lines in your changes missing coverage. Please review.

Project coverage is 81.75%. Comparing base (2971b8a) to head (e051836).
Report is 20 commits behind head on master.

Files with missing lines	Patch %	Lines
cli/server/server.go	15.90%	148 Missing ⚠️
pkg/core/native/management.go	0.00%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3960      +/-   ##
==========================================
- Coverage   82.03%   81.75%   -0.29%     
==========================================
  Files         348      348              
  Lines       49222    49419     +197     
==========================================
+ Hits        40380    40402      +22     
- Misses       7147     7329     +182     
+ Partials     1695     1688       -7

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ixje · 2025-07-21T15:39:28Z

I overlooked downloading the contract storage 🙈 . Still open to input on the TODOs how to best expose the data. Consider this a draft (I don't have the option to change it to one)

AnnaShaleva

Good feature, but some critical decisions need to be polished.

AnnaShaleva · 2025-07-22T07:41:00Z

cli/server/server.go

+		&cli.UintFlag{
+			Name:        "height",
+			Usage:       "Height at which to get the contract state for",
+			Required:    false,
+			DefaultText: "latest",
+		},


We have options.Historic designed specifically for such kind of things.

AnnaShaleva · 2025-07-22T07:45:11Z

cli/server/server.go

+				{
+					Name:      "download-contract",
+					Usage:     "Download a contract including storage from a remote chain into the local database",
+					UsageText: "neo-go db download-contract -c contract-hash -r endpoint [--height height] [--config-file] [--force]",
+					Action:    downloadContract,
+					Flags:     cfgDownloadFlags,
+				},


I'd say that this command is closer to contract subset of commands (e.g. ./neo-go contract download). Also, it reminds me of ./neo-go wallet import-deployed, so for the sake of CLI interface unification we can use it as ./neo-go contract import-deployed. But it's a matter of discussion because, as @ixje said, this commend performs a direct DB modification.

@roman-khimov?

performs a direct DB modification

Which makes it a db command for me. But I'm thinking of separating download and import stages (consider contract state as well). In which case contract download + db import-contract would be a nice pair. This separation can be useful for unit tests similar to ones we have in neofs-contract repository where some state is stored in the repository and imported as needed for tests.

This needs to be discussed, it is possible to unify these tools and we better do this now, before we have 25 variations on the same thing.

Vote up for separation.

AnnaShaleva · 2025-07-22T07:53:36Z

cli/server/server.go

+		&cli.StringFlag{
+			Name:     "contract-hash",
+			Usage:    "Script hash of the contract to download",
+			Required: true,
+			Aliases:  []string{"c"},
+		},


It should be of flags.Address type. We already have a designated flag for this, it can be moved to options package and reused from both wallet and contract:

neo-go/cli/wallet/wallet.go

Lines 300 to 305 in 4340c76

&flags.AddressFlag{

Name: "contract",

Aliases: []string{"c"},

Required: true,

Usage: "Contract hash or address",

},

AnnaShaleva · 2025-07-22T07:57:42Z

cli/server/server.go

+	ch, err := util.Uint160DecodeStringLE(ctx.String("contract-hash")[2:])
+	if err != nil {
+		return cli.Exit(fmt.Errorf("failed to decode contract hash: %v", err), 1)
+	}


This code will be simplified after flag type replacement.

AnnaShaleva · 2025-07-22T08:19:38Z

cli/server/server.go

+		_, err := native.GetContractByID(d, contractState.ID)
+		if !errors.Is(err, storage.ErrKeyNotFound) {
+			// ID is in use, get a new one


We need to check two cases: if err == nil means that ID is occupied, we can take the next free one. if err != nil && !errors.Is(err, storage.ErrKeyNotFound) means that it's likely some internal DB error, we should exit in this case.

AnnaShaleva · 2025-07-22T09:24:40Z

cli/server/server.go

+	prefix := append([]byte{0x08}, hash.BytesBE()...)
+	states, err := client.FindStates(
+		stateResponse.Root,
+		managementContract,
+		prefix,
+		nil,
+		nil,
+	)
+	if err != nil {


... we can get rid of this code. Just invoke native Management's getContract via historic Invoker, that's it.

Using internal knowledge of native Management storage like []byte{0x08} at upper-level modules is not the best idea, it should be avoided if we can. The drawback of the suggested approach is: it won't work with C# node, they don't support historic invocations. @ixje is it critical?

Critical no, but might be inconvenient as now we'll need to add some graceful handling if the RPC node doesn't support historic fetching.

How about adding a comment explaining the prefix used while maintaining support for both types of nodes? It is not as if this is likely to ever change

OK, let's stick to the implementation-agnostic solution.

How about adding a comment

Agree, it's needed.

Also, let's replace magic 0x08 byte with native.PrefixContract constant, so that it will be more clear to other developers.

AnnaShaleva · 2025-07-22T09:25:10Z

cli/server/server.go

+		return nil, fmt.Errorf(
+			"failed to fetch contract state for contract %s: %w",
+			hash.StringLE(),
+			err,
+		)


Use single line.

AnnaShaleva · 2025-07-22T09:25:22Z

cli/server/server.go

+func GetContractStateAtHeight(
+	client *rpcclient.Client,
+	height uint32,
+	hash util.Uint160,
+) (*state.Contract, error) {


Use single line for this.

AnnaShaleva · 2025-07-22T09:26:45Z

pkg/core/native/management.go

+// TODO: check with nspcc if we can update PutContractState with an extra param
+// or else how to have the cache for ManagementContract initialized such that it doesn't panic
+// trying to run markUpdated()
+func PutContractStateNoCache(d *dao.Simple, cs *state.Contract) error {
+	return putContractState(d, cs, false)
+}


See the comment above at downloadContract.

AnnaShaleva · 2025-07-22T09:30:57Z

pkg/core/native/management.go

+
+// TODO: decide how to best expose. There are too many options that for the time I picked a duplicate
+// function to have something working and let nspcc decide their preferred exposing option
+func GetNextContractID(d *dao.Simple) (int32, error) {


Let's firstly decide where to place DB modification code.

ixje · 2025-07-22T09:53:37Z

Just now seeing the feedback. Will update after lunch

ixje · 2025-07-22T10:53:26Z

#3960 (comment)
This needs to be discussed, it is possible to unify these tools and we better do this now, before we have 25 variations on the same thing.

I'm going to refrain from further updates to the PR until you've internally decided what the next move is before I get to throw away significant parts.

AnnaShaleva · 2025-08-05T12:24:51Z

Discussed, let's restructure the code in the following way:

contract download command that will iterate through the historic contract storage via findstates RPC (this code is already present in the current PR, it just should be moved to a separate command) and dump the full contract state and contract storage to a file.
The dump format used by https://github.com/nspcc-dev/neofs-contract/blob/63a8deaafd21ce741d026e3e3973093596e41932/tests/dump/creator.go#L20 is JSON for contract states + CSV for contract storage. However, given the fact that in NeoGo we'll use contract download to download a single contract per command run, to me it looks more native to flush state+storage pair to a single JSON file in the following form:

{
    "contract": {/*JSON-ized state.Contract bytes*/},
    "storage": {/*the list of serialized KV-pairs in base64*/}
}

This representation differs from the one used by NeoFS contracts. @roman-khimov, ACK?
3. db import-contract command that will read contract/storage from file and insert it into the blockchain. This code is also written in this PR and just needs to be transferred to a proper place. Regarding direct DB modifications (#3960 (comment)): let's follow the approach implemented in https://github.com/nspcc-dev/neofs-contract/blob/63a8deaafd21ce741d026e3e3973093596e41932/tests/migration/storage.go#L78, it solves the problem without direct Blockchain modifications. Once #2926 is done, we'll be able to migrate onto less invasive approach.

@ixje, if there are some additional questions, we may setup a meeting call and discuss them in person.

roman-khimov · 2025-08-05T12:34:53Z

This representation differs from the one used by NeoFS contracts. @roman-khimov, ACK?

Seems to be easy to adopt. The only concern is contracts with huge state, CSV is easy to iterate over and process entry by entry, JSON is different.

AnnaShaleva · 2025-08-05T12:59:47Z

CSV is easy to iterate over and process entry by entry, JSON is different.

Agreed to use split format: JSON for contract states + CSV for storage (a raw set of key-value pairs without contract identifier).

ixje · 2025-08-05T17:14:18Z

fyi; I'm about to go on a holiday, so won't be able to pick this up until late August.

ixje added 3 commits July 21, 2025 14:56

initial

691aace

use local contract ID if already exists by hash

e388839

undo

c276454

ixje requested review from AnnaShaleva and roman-khimov as code owners July 21, 2025 13:31

AnnaShaleva reviewed Jul 22, 2025

View reviewed changes

also fetch & store contract storages

75ae114

process some feedback

e051836

	&flags.AddressFlag{
	Name: "contract",
	Aliases: []string{"c"},
	Required: true,
	Usage: "Contract hash or address",
	},

cli: db download-contract command #3960

Are you sure you want to change the base?

cli: db download-contract command #3960

Uh oh!

Conversation

ixje commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

open things to address

Uh oh!

roman-khimov commented Jul 21, 2025

Uh oh!

codecov bot commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ixje commented Jul 21, 2025

Uh oh!

AnnaShaleva left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ixje Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ixje commented Jul 22, 2025

Uh oh!

ixje commented Jul 22, 2025

Uh oh!

AnnaShaleva commented Aug 5, 2025

Uh oh!

roman-khimov commented Aug 5, 2025

Uh oh!

AnnaShaleva commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ixje commented Aug 5, 2025

Uh oh!

Uh oh!

ixje commented Jul 21, 2025 •

edited

Loading

codecov bot commented Jul 21, 2025 •

edited

Loading

ixje Jul 22, 2025 •

edited

Loading

AnnaShaleva commented Aug 5, 2025 •

edited

Loading