Integrate frostfsid into NNS #119

Merged
fyrchik merged 3 commits from achuprov/frostfs-contract:feat/integrate_frostfsid2nns into master 2024-11-05 14:58:57 +00:00
8 changed files with 176 additions and 3 deletions

View file

@ -96,6 +96,7 @@ const (
createSubjectMethod = "createSubject"
getSubjectMethod = "getSubject"
getSubjectExtendedMethod = "getSubjectExtended"
getSubjectKVMethod = "getSubjectKV"
listSubjectsMethod = "listSubjects"
addSubjectKeyMethod = "addSubjectKey"
removeSubjectKeyMethod = "removeSubjectKey"
@ -266,6 +267,11 @@ func (c Client) GetSubjectExtended(addr util.Uint160) (*SubjectExtended, error)
return ParseSubjectExtended(items)
}
// GetSubjectKV invokes `getSubjectKV` method of contract.
func (c Client) GetSubjectKV(addr util.Uint160, name string) (string, error) {
return unwrap.UTF8String(c.act.Call(c.contract, getSubjectKVMethod, addr, name))
dkirillov marked this conversation as resolved Outdated

It seems we can write just

return unwrap.UTF8String(c.act.Call(c.contract, getSubjectKVMethod, addr, name))
It seems we can write just ```golang return unwrap.UTF8String(c.act.Call(c.contract, getSubjectKVMethod, addr, name)) ```

fixed

fixed
}
// ListSubjects gets all subjects.
func (c Client) ListSubjects() ([]util.Uint160, error) {
return UnwrapArrayOfUint160(commonclient.ReadIteratorItems(c.act, iteratorBatchSize, c.contract, listSubjectsMethod))

View file

@ -7,6 +7,7 @@ safemethods:
- "getGroupByName"
- "getNamespace"
- "getNamespaceExtended"
- "getSubjectKV"
- "getSubject"
- "getSubjectExtended"
- "getSubjectByKey"

View file

@ -472,6 +472,34 @@ func GetSubjectKeyByName(ns, name string) interop.PublicKey {
return subjKey
}
// GetSubjectKV GetSubjectKey returns the value associated with the key for the subject.
func GetSubjectKV(addr interop.Hash160, name string) string {
dkirillov marked this conversation as resolved Outdated

I suggest to rename this function to GetSubjectKV

I suggest to rename this function to `GetSubjectKV`

Renamed

Renamed
if len(addr) != interop.Hash160Len {
panic("incorrect address length")
}
ctx := storage.GetReadOnlyContext()
sKey := subjectKeyFromAddr(addr)
data := storage.Get(ctx, sKey).([]byte)
if data == nil {
return ""
}
sbj := std.Deserialize(data).(Subject)
if sbj.KV == nil {
return ""
}
for k, v := range sbj.KV {
if k == name {
return v
}
}
return ""
}
func ListSubjects() iterator.Iterator {
ctx := storage.GetReadOnlyContext()
return storage.Find(ctx, []byte{subjectKeysPrefix}, storage.KeysOnly|storage.RemovePrefix)

View file

@ -44,3 +44,32 @@ The committee makes new tokens (domains), sets, and charges a fee for issuance.
## Globally Unique Domain Zone
For more information, see [here](../docs/globally-unique-domain-zone.md).
## NNS and Frostfsid
You can register a TLD domain without a committee signature using Frostfsid. To do this, create a new wallet
```
neo-go wallet init -w newwallet/wallet.json
```
Get wallet address:
```
neo-go wallet dump-keys -w newwallet/wallet.json
[subject-address]
[subject-key]
```
Create a subject in `FrostfsID`:
```
frostfs-adm morph frostfsid create-subject --subject-key="[subject-key]"
```
Grant permissions to the wallet:
```
frostfs-adm morph nns give-privilege --subject-address="[subject-address]"
```
Register domain:
```
neo-go contract invokefunction [NNS-hash] register "subdomain.domain" hash160:[subject-address] "email@frostfs.info" 10000 1000 1000 1000 -- [subject-address]:Global
```

39
nns/frostfsid.go Normal file
View file

@ -0,0 +1,39 @@
package nns
import (
"github.com/nspcc-dev/neo-go/pkg/interop"
"github.com/nspcc-dev/neo-go/pkg/interop/contract"
"github.com/nspcc-dev/neo-go/pkg/interop/native/management"
"github.com/nspcc-dev/neo-go/pkg/interop/native/std"
"github.com/nspcc-dev/neo-go/pkg/interop/storage"
)
const FrostfsIDNNSName = "frostfsid.frostfs"
const (
FrostfsIDNNSTLDPermissionKey = "nns-allow-register-tld"
FrostfsIDTLDRegistrationAllowed = "allow"
)

It is used only in tests, could you remove it?
We explicitly do allow check, everything else should be disallow.

It is used only in tests, could you remove it? We explicitly do allow check, _everything_ else should be disallow.
func checkFrostfsID(ctx storage.Context, addr interop.Hash160) bool {
if len(addr) == 0 {

We can store address in base-58 and use 29bb3ff1cf/pkg/core/native/std.go (L108)

We can store address in base-58 and use https://github.com/nspcc-dev/neo-go/blob/29bb3ff1cf4738ada24f075f5b398c1e70522778/pkg/core/native/std.go#L108

See #96

See https://git.frostfs.info/TrueCloudLab/frostfs-contract/issues/96

fixed

fixed
return false
}
frostfsIDAddress := getRecordsByType(ctx, []byte(tokenIDFromName(FrostfsIDNNSName)), FrostfsIDNNSName, TXT)
if len(frostfsIDAddress) < 2 {
return false
dkirillov marked this conversation as resolved Outdated

Should this be

if len(frostfsIDAddress) < 2 {

because below we use frostfsIDAddress[1]
?

Should this be ```golang if len(frostfsIDAddress) < 2 { ``` because below we use `frostfsIDAddress[1]` ?

Do we have a strict requirement to have the address in both Base56 and hex formats? The hex format does not affect functionality.

Do we have a strict requirement to have the address in both `Base56` and `hex` formats? The `hex` format does not affect functionality.

My comment was just about avoiding panic when result will contain just one element and we will try to access the second (frostfsIDAddress[1])

My comment was just about avoiding panic when result will contain just one element and we will try to access the second (`frostfsIDAddress[1]`)

You're right. Fixed.

You're right. Fixed.

I'm not sure. It seems you fixed wrong line

I'm not sure. It seems you fixed wrong line
}

Each signer has Scope, that is "bytecode area", where his signature is considered valid.
I may be OK with paying for this transaction (use None scope), but not ok with using my permissions to allow SOMEONE to do whatever they want.

Each signer has `Scope`, that is "bytecode area", where his signature is considered valid. I may be OK with paying for this transaction (use `None` scope), but not ok with using my permissions to allow SOMEONE to do whatever they want.

Another problem with this line of code is multiple contract.Call() invocations. This is too costly.

Another problem with this line of code is multiple `contract.Call()` invocations. This is too costly.

I think domain owner should be used here. He will own the domain, he should have the right to register.

I think domain owner should be used here. He will own the domain, he should have the right to register.

Each signer has Scope, that is "bytecode area", where his signature is considered valid.
I may be OK with paying for this transaction (use None scope), but not ok with using my permissions to allow SOMEONE to do whatever they want.

Just for my curiosity. You've said: "use None scope". Will be the contract method called then? I am looking at "getValueForSubjectKey" definition and it's expected that the method is invoked in the read context. So, why can't we use ReadOnly | AllowCall flags then?

> Each signer has `Scope`, that is "bytecode area", where his signature is considered valid. > I may be OK with paying for this transaction (use `None` scope), but not ok with using my permissions to allow SOMEONE to do whatever they want. Just for my curiosity. You've said: "use `None` scope". Will be the contract method called then? I am looking at `"getValueForSubjectKey"` definition and it's expected that the method is invoked in the read context. So, why can't we use `ReadOnly | AllowCall` flags then?

Signer scope (None, CalledByEntry etc) is different from the call flags (ReadOnly, AllowCall, etc).

Signer scope (`None`, `CalledByEntry` etc) is different from the call flags (`ReadOnly`, `AllowCall`, etc).

fixed

fixed
decodedBytes := std.Base58Decode([]byte(frostfsIDAddress[1]))
if len(decodedBytes) < 21 || management.GetContract(decodedBytes[1:21]) == nil {
return false
}
if res := contract.Call(decodedBytes[1:21], "getSubjectKV", contract.ReadOnly, addr, FrostfsIDNNSTLDPermissionKey).(string); res == FrostfsIDTLDRegistrationAllowed {
return true
}
return false
}

View file

@ -344,7 +344,7 @@ func register(ctx storage.Context, name string, owner interop.Hash160, email str
tldBytes := storage.Get(ctx, tldKey)
if countZone == 1 {
checkCommittee()
checkCommitteeAndFrostfsID(ctx, owner)
if tldBytes != nil {
panic("TLD already exists")
}
@ -924,6 +924,14 @@ func isValid(address interop.Hash160) bool {
return address != nil && len(address) == interop.Hash160Len
}
// checkCommitteeAndFrostfsID panics if the script container is not signed by the committee.
// or if the owner does not have permission in FrostfsID.
func checkCommitteeAndFrostfsID(ctx storage.Context, owner interop.Hash160) {
if !checkFrostfsID(ctx, owner) {
checkCommittee()
}
}
// checkCommittee panics if the script container is not signed by the committee.
func checkCommittee() {
committee := neo.GetCommittee()

View file

@ -222,6 +222,11 @@ func (c *ContractReader) GetSubjectExtended(addr util.Uint160) ([]stackitem.Item
return unwrap.Array(c.invoker.Call(c.hash, "getSubjectExtended", addr))
}
// GetSubjectKV invokes `getSubjectKV` method of contract.
func (c *ContractReader) GetSubjectKV(addr util.Uint160, name string) (string, error) {
return unwrap.UTF8String(c.invoker.Call(c.hash, "getSubjectKV", addr, name))
}
// GetSubjectKeyByName invokes `getSubjectKeyByName` method of contract.
func (c *ContractReader) GetSubjectKeyByName(ns string, name string) (*keys.PublicKey, error) {
dkirillov marked this conversation as resolved Outdated

New method should also be added to frsotfsid/client/client.go file

New method should also be added to `frsotfsid/client/client.go` file
return unwrap.PublicKey(c.invoker.Call(c.hash, "getSubjectKeyByName", ns, name))

View file

@ -12,10 +12,12 @@ import (
"github.com/nspcc-dev/neo-go/pkg/core/interop/storage"
"github.com/nspcc-dev/neo-go/pkg/core/state"
"github.com/nspcc-dev/neo-go/pkg/core/transaction"
"github.com/nspcc-dev/neo-go/pkg/encoding/address"
"github.com/nspcc-dev/neo-go/pkg/neotest"
"github.com/nspcc-dev/neo-go/pkg/rpcclient/gas"
"github.com/nspcc-dev/neo-go/pkg/util"
"github.com/nspcc-dev/neo-go/pkg/vm/stackitem"
"github.com/nspcc-dev/neo-go/pkg/wallet"
"github.com/stretchr/testify/require"
)
@ -23,8 +25,7 @@ const nnsPath = "../nns"
const msPerYear = 365 * 24 * time.Hour / time.Millisecond
func newNNSInvoker(t *testing.T, addRoot bool) *neotest.ContractInvoker {
e := newExecutor(t)
func deployNNS(t *testing.T, e *neotest.Executor, addRoot bool) *neotest.ContractInvoker {
ctr := neotest.CompileFile(t, e.CommitteeHash, nnsPath, path.Join(nnsPath, "config.yml"))
e.DeployContract(t, ctr, nil)
@ -39,6 +40,28 @@ func newNNSInvoker(t *testing.T, addRoot bool) *neotest.ContractInvoker {
return c
}
func newNNSInvoker(t *testing.T, addRoot bool) *neotest.ContractInvoker {
e := newExecutor(t)
return deployNNS(t, e, addRoot)
}
func newNNSInvokerWithFrostfsID(t *testing.T, addRoot bool) (*neotest.ContractInvoker, *neotest.ContractInvoker) {
e := newExecutor(t)
c := deployNNS(t, e, addRoot)
frostfdID := deployFrostFSIDContract(t, e, e.CommitteeHash)
c.Invoke(t, true, "register",
nns.FrostfsIDNNSName, c.CommitteeHash,
"myemail@frostfs.info", defaultRefresh, defaultRetry, defaultExpire, defaultTTL)
c.Invoke(t, stackitem.Null{}, "addRecord",
nns.FrostfsIDNNSName, int64(nns.TXT), frostfdID.StringLE())
c.Invoke(t, stackitem.Null{}, "addRecord",
nns.FrostfsIDNNSName, int64(nns.TXT), address.Uint160ToString(frostfdID))

Please, let's not overcomplicate function signatures.
newNNSInvoker is perfectly valid.
We might have another newNNSInvokerWithFrostfsID which reuses newNNSInvoker and do not change unaffected tests. We need it only once (!).

Please, let's not overcomplicate function signatures. `newNNSInvoker` is perfectly valid. We might have another `newNNSInvokerWithFrostfsID` which reuses `newNNSInvoker` and do not change unaffected tests. We need it only once (!).

fixed

fixed
return e.NewInvoker(c.Hash), e.CommitteeInvoker(frostfdID)
}
func TestNNSGeneric(t *testing.T) {
c := newNNSInvoker(t, false)
@ -527,6 +550,40 @@ func TestNNSSetAdmin(t *testing.T) {
"testdomain.com", int64(nns.TXT), "will be added")
}
func TestNNS_Frostfsid(t *testing.T) {
nnsInv, f := newNNSInvokerWithFrostfsID(t, false)
acc, err := wallet.NewAccount()
require.NoError(t, err)
nnsUserInv := nnsInv.NewInvoker(nnsInv.Hash, newSigner(t, nnsInv, acc))
nnsUserInv.InvokeFail(t, "not witnessed by committee", "register",
"ddd", acc.ScriptHash(),
"myemail@frostfs.info", defaultRefresh, defaultRetry, defaultExpire, defaultTTL)
nnsUserInv.InvokeFail(t, "not witnessed by committee", "register",
"testdomain.kz", acc.ScriptHash(),
"myemail@frostfs.info", defaultRefresh, defaultRetry, defaultExpire, defaultTTL)
f.Invoke(t, stackitem.Null{}, createSubjectMethod, "", acc.PrivateKey().PublicKey().Bytes())
nnsUserInv.InvokeFail(t, "not witnessed by committee", "register",
"testdomain.kz", acc.ScriptHash(),
"myemail@frostfs.info", defaultRefresh, defaultRetry, defaultExpire, defaultTTL)
f.Invoke(t, stackitem.Null{}, setSubjectKVMethod, acc.ScriptHash(), nns.FrostfsIDNNSTLDPermissionKey, nns.FrostfsIDTLDRegistrationAllowed)
nnsUserInv.Invoke(t, true, "register",
"testdomain.kz", acc.ScriptHash(),
"myemail@frostfs.info", defaultRefresh, defaultRetry, defaultExpire, defaultTTL)
f.Invoke(t, stackitem.Null{}, deleteSubjectKVMethod, acc.ScriptHash(), nns.FrostfsIDNNSTLDPermissionKey)
nnsUserInv.InvokeFail(t, "not witnessed by committee", "register",
"testdomain.uz", acc.ScriptHash(),
"myemail@frostfs.info", defaultRefresh, defaultRetry, defaultExpire, defaultTTL)
}
func TestNNSIsAvailable(t *testing.T) {
c := newNNSInvoker(t, false)