Append SystemContractHash function

[smallhive]

Including contract RPC wrappers in code led to having the same function in a few projects:

// SystemContractHash allows to get system contract hash.
func SystemContractHash(cl *rpcclient.Client, id int32) (util.Uint160, error) {
    c, err := cl.GetContractStateByID(id)
    if err != nil {
        return util.Uint160{}, fmt.Errorf("GetContractStateByID [%d]: %w", id, err)
    }

    return c.Hash, nil
}
/// ...
const nnsContract = int32(1)

nnsHash, err := SystemContractHash(cl, nnsContract)
inv := invoker.New(cl, nil)
nnsReader := rpcNNS.NewReader(inv, nnsHash)
/// ...

It would be good, to have such code inside the contracts repo with the actual list of system contracts.

The code is small, but it is a copy-paste for many repos

[roman-khimov]

I'd say we need two things:

func NNSContractHash(cl *rpcclient.Client) (util.Uint160, error)
func (c *NNSReader) ResolveNeoFSContract(name string) (util.Uint160, error)

The second one is a rather obvious manual addition to the nns package, the first one is not so much, but maybe can be stuffed into nns as well (nns.AutoResolveContractHash?).

@AnnaShaleva?

[roman-khimov]

Or just nns.NewAutoHash() and nns.NewReaderAutoHash() for the first one.

[AnnaShaleva]

The second one is a rather obvious manual addition to the nns package

If possible, I'd like to avoid manual code addition to auto-generated RPC bindings. If this method is needed to be present inside the NNS RPC wrapper, then we need to add this method to the NNS contract itself, and after that regenerate bindings one more time.

Otherwise, these helpers look good to me, we definitely need them.

[AnnaShaleva]

Or maybe for func NNSContractHash(cl *rpcclient.Client) (util.Uint160, error) we have to create a separate helper package in order not to modify the auto-generated code.

[roman-khimov]

helper, util or libmisc is the thing I'm specifically trying to avoid here. What we have here is a set of conventions, NeoFS sets up and uses NNS contract in some specific way and every component around is relying on these conventions. Right now every component duplicates this magic knowledge internally at the same time. What we want is have some common place for them.

nns is an auto-generated wrapper, but we can add any additional code into it (just like in #347). I agree that keeping it clean would probably be better long-term, yet at the same time we need some place for these things then. What other conventions do we have? Can some neofschain be created with a meaningful set of functions (especially if some are not contract-bound) or we better stick to per-contract additions? @carpawell?

[AnnaShaleva]

Can some neofschain be created with a meaningful set of functions (especially if some are not contract-bound) or we better stick to per-contract additions?

There likely to exist a tiny set of other helper functions that can also be included into this neofschain helper package. And still, if there are, then it's better to create this package. But if not, then I vote for adding an automatic script that modifies auto-generated RPC bindings in order not to perform these additions manually each time when make all is called.

In general, I'm against any manual modifications in the auto-generated code. If it's done with some script (and added to our Makefile), then it's OK to me (but still less preferable than a separate helper package).

[AnnaShaleva]

Can some neofschain be created with a meaningful set of functions

How about neofs utility that reads contracts (it's contract-related, but still)? Currently it's located in the https://github.com/nspcc-dev/neofs-node/blob/master/contracts/contracts.go, but maybe it can be moved to this neofschain package?

[AnnaShaleva]

@roman-khimov, I took a look at the https://github.com/nspcc-dev/neofs-contract/pull/347 one more time, and I've missed that this RecordType structure is added to the /rpc/nns package and is extending the auto-generated binding. This code looks good and causes no problems on bindings regeneration, thus, I think we can keep it and follow the same scheme on further manual RPC bindings extension. I don't think that we're going to have a lot of them.