From 43097d2152d34236261eb674d03d8152886d23ae Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Tue, 28 Nov 2023 11:47:12 +0300 Subject: [PATCH] [#55] frostfsid: Use single admin instead of many Autorization can be dedicated to a separate contract, iterating over multiple keys can be costly. Also add committee as "default" admin: everything is allowed for it. Signed-off-by: Evgenii Stratonikov --- frostfsid/client/client.go | 49 +++++++++----- frostfsid/config.yml | 4 +- frostfsid/frostfsid_contract.go | 40 +++++------ rpcclient/frostfsid/client.go | 115 ++++++++++++++------------------ tests/frostfsid_client_test.go | 23 ++++--- tests/frostfsid_test.go | 52 ++++++++++----- 6 files changed, 149 insertions(+), 134 deletions(-) diff --git a/frostfsid/client/client.go b/frostfsid/client/client.go index 570df8a..07ef08b 100644 --- a/frostfsid/client/client.go +++ b/frostfsid/client/client.go @@ -82,9 +82,9 @@ const ( const iteratorBatchSize = 100 const ( - addOwnerMethod = "addOwner" - deleteOwnerMethod = "deleteOwner" - listOwnersMethod = "listOwners" + getAdminMethod = "getAdmin" + setAdminMethod = "setAdmin" + clearAdminMethod = "clearAdmin" versionMethod = "version" @@ -157,33 +157,46 @@ func (c Client) Version() (int64, error) { return unwrap.Int64(c.act.Call(c.contract, versionMethod)) } -// AddOwner adds new address that can perform write operations on contract. +// SetAdmin sets address that can perform write operations on contract. // Must be invoked by committee. -func (c Client) AddOwner(owner util.Uint160) (tx util.Uint256, vub uint32, err error) { - method, args := c.AddOwnerCall(owner) +func (c Client) SetAdmin(owner util.Uint160) (tx util.Uint256, vub uint32, err error) { + method, args := c.SetAdminCall(owner) return c.act.SendCall(c.contract, method, args...) } -// AddOwnerCall provides args for AddOwner to use in commonclient.Transaction. -func (c Client) AddOwnerCall(owner util.Uint160) (method string, args []any) { - return addOwnerMethod, []any{owner} +// SetAdminCall provides args for SetAdmin to use in commonclient.Transaction. +func (c Client) SetAdminCall(owner util.Uint160) (method string, args []any) { + return setAdminMethod, []any{owner} } -// DeleteOwner removes address from list of that can perform write operations on contract. +// ClearAdmin removes address that can perform write operations on contract. // Must be invoked by committee. -func (c Client) DeleteOwner(owner util.Uint160) (tx util.Uint256, vub uint32, err error) { - method, args := c.DeleteOwnerCall(owner) +func (c Client) ClearAdmin() (tx util.Uint256, vub uint32, err error) { + method, args := c.ClearAdminCall() return c.act.SendCall(c.contract, method, args...) } -// DeleteOwnerCall provides args for DeleteOwner to use in commonclient.Transaction. -func (c Client) DeleteOwnerCall(owner util.Uint160) (method string, args []any) { - return deleteOwnerMethod, []any{owner} +// ClearAdminCall provides args for ClearAdmin to use in commonclient.Transaction. +func (c Client) ClearAdminCall() (method string, args []any) { + return clearAdminMethod, nil } -// ListOwners returns list of address that can perform write operations on contract. -func (c Client) ListOwners() ([]util.Uint160, error) { - return unwrapArrayOfUint160(commonclient.ReadIteratorItems(c.act, iteratorBatchSize, c.contract, listOwnersMethod)) +// GetAdmin returns address that can perform write operations on contract. +// Second return values is true iff admin is set. +func (c Client) GetAdmin() (util.Uint160, bool, error) { + item, err := unwrap.Item(c.act.Call(c.contract, getAdminMethod)) + if err != nil { + return util.Uint160{}, false, err + } + if item.Value() == nil { + return util.Uint160{}, false, nil + } + bs, err := item.TryBytes() + if err != nil { + return util.Uint160{}, true, err + } + u, err := util.Uint160DecodeBytesBE(bs) + return u, true, err } // CreateSubject creates new subject using public key. diff --git a/frostfsid/config.yml b/frostfsid/config.yml index 36d304d..eeaec4d 100644 --- a/frostfsid/config.yml +++ b/frostfsid/config.yml @@ -1,5 +1,7 @@ name: "Identity" -safemethods: ["version"] +safemethods: + - "getAdmin" + - "version" permissions: - methods: ["update"] events: diff --git a/frostfsid/frostfsid_contract.go b/frostfsid/frostfsid_contract.go index 98f6fdc..0f391ad 100644 --- a/frostfsid/frostfsid_contract.go +++ b/frostfsid/frostfsid_contract.go @@ -58,7 +58,7 @@ type ( ) const ( - ownerKeysPrefix = 'o' + adminKey = 'o' subjectKeysPrefix = 's' additionalKeysPrefix = 'a' namespaceKeysPrefix = 'n' @@ -74,14 +74,14 @@ func _deploy(data any, isUpdate bool) { ctx := storage.GetContext() args := data.(struct { - owners []interop.Hash160 + admin interop.Hash160 }) - for _, owner := range args.owners { - if len(owner) != interop.Hash160Len { - panic("incorrect length of owner addresses") + if args.admin != nil { + if len(args.admin) != interop.Hash160Len { + panic("incorrect length of owner address") } - storage.Put(ctx, ownerKey(owner), []byte{1}) + storage.Put(ctx, adminKey, args.admin) } storage.Put(ctx, groupCounterKey, 0) @@ -89,27 +89,27 @@ func _deploy(data any, isUpdate bool) { runtime.Log("frostfsid contract initialized") } -func AddOwner(addr interop.Hash160) { +func SetAdmin(addr interop.Hash160) { ctx := storage.GetContext() if !common.HasUpdateAccess() { panic("not witnessed") } - storage.Put(ctx, ownerKey(addr), []byte{1}) + storage.Put(ctx, adminKey, addr) } -func DeleteOwner(addr interop.Hash160) { +func ClearAdmin() { ctx := storage.GetContext() if !common.HasUpdateAccess() { panic("not witnessed") } - storage.Delete(ctx, ownerKey(addr)) + storage.Delete(ctx, adminKey) } -func ListOwners() iterator.Iterator { +func GetAdmin() interop.Hash160 { ctx := storage.GetReadOnlyContext() - return storage.Find(ctx, []byte{ownerKeysPrefix}, storage.KeysOnly|storage.RemovePrefix) + return storage.Get(ctx, adminKey).(interop.Hash160) } // Update method updates contract source code and manifest. It can be invoked @@ -815,12 +815,12 @@ func DeleteGroup(ns string, groupID int) { } func checkContractOwner(ctx storage.Context) { - it := storage.Find(ctx, []byte{ownerKeysPrefix}, storage.KeysOnly|storage.RemovePrefix) - for iterator.Next(it) { - owner := iterator.Value(it).([]byte) - if runtime.CheckWitness(owner) { - return - } + addr := storage.Get(ctx, adminKey) + if addr != nil && runtime.CheckWitness(addr.(interop.Hash160)) { + return + } + if common.HasUpdateAccess() { + return } panic("not witnessed") } @@ -905,10 +905,6 @@ func setNamespaceGroupName(ctx storage.Context, gr Group) { } } -func ownerKey(owner interop.Hash160) []byte { - return append([]byte{ownerKeysPrefix}, owner...) -} - func subjectKey(key interop.PublicKey) []byte { addr := contract.CreateStandardAccount(key) return subjectKeyFromAddr(addr) diff --git a/rpcclient/frostfsid/client.go b/rpcclient/frostfsid/client.go index 99d04a2..c210329 100644 --- a/rpcclient/frostfsid/client.go +++ b/rpcclient/frostfsid/client.go @@ -163,33 +163,16 @@ func New(actor Actor, hash util.Uint160) *Contract { return &Contract{ContractReader{actor, hash}, actor, hash} } +// GetAdmin invokes `getAdmin` method of contract. +func (c *ContractReader) GetAdmin() (util.Uint160, error) { + return unwrap.Uint160(c.invoker.Call(c.hash, "getAdmin")) +} + // Version invokes `version` method of contract. func (c *ContractReader) Version() (*big.Int, error) { return unwrap.BigInt(c.invoker.Call(c.hash, "version")) } -// AddOwner creates a transaction invoking `addOwner` method of the contract. -// This transaction is signed and immediately sent to the network. -// The values returned are its hash, ValidUntilBlock value and error if any. -func (c *Contract) AddOwner(addr util.Uint160) (util.Uint256, uint32, error) { - return c.actor.SendCall(c.hash, "addOwner", addr) -} - -// AddOwnerTransaction creates a transaction invoking `addOwner` method of the contract. -// This transaction is signed, but not sent to the network, instead it's -// returned to the caller. -func (c *Contract) AddOwnerTransaction(addr util.Uint160) (*transaction.Transaction, error) { - return c.actor.MakeCall(c.hash, "addOwner", addr) -} - -// AddOwnerUnsigned creates a transaction invoking `addOwner` method of the contract. -// This transaction is not signed, it's simply returned to the caller. -// Any fields of it that do not affect fees can be changed (ValidUntilBlock, -// Nonce), fee values (NetworkFee, SystemFee) can be increased as well. -func (c *Contract) AddOwnerUnsigned(addr util.Uint160) (*transaction.Transaction, error) { - return c.actor.MakeUnsignedCall(c.hash, "addOwner", nil, addr) -} - // AddSubjectKey creates a transaction invoking `addSubjectKey` method of the contract. // This transaction is signed and immediately sent to the network. // The values returned are its hash, ValidUntilBlock value and error if any. @@ -256,6 +239,28 @@ func (c *Contract) AddSubjectToNamespaceUnsigned(addr util.Uint160, ns string) ( return c.actor.MakeUnsignedCall(c.hash, "addSubjectToNamespace", nil, addr, ns) } +// ClearAdmin creates a transaction invoking `clearAdmin` method of the contract. +// This transaction is signed and immediately sent to the network. +// The values returned are its hash, ValidUntilBlock value and error if any. +func (c *Contract) ClearAdmin() (util.Uint256, uint32, error) { + return c.actor.SendCall(c.hash, "clearAdmin") +} + +// ClearAdminTransaction creates a transaction invoking `clearAdmin` method of the contract. +// This transaction is signed, but not sent to the network, instead it's +// returned to the caller. +func (c *Contract) ClearAdminTransaction() (*transaction.Transaction, error) { + return c.actor.MakeCall(c.hash, "clearAdmin") +} + +// ClearAdminUnsigned creates a transaction invoking `clearAdmin` method of the contract. +// This transaction is not signed, it's simply returned to the caller. +// Any fields of it that do not affect fees can be changed (ValidUntilBlock, +// Nonce), fee values (NetworkFee, SystemFee) can be increased as well. +func (c *Contract) ClearAdminUnsigned() (*transaction.Transaction, error) { + return c.actor.MakeUnsignedCall(c.hash, "clearAdmin", nil) +} + // CreateGroup creates a transaction invoking `createGroup` method of the contract. // This transaction is signed and immediately sent to the network. // The values returned are its hash, ValidUntilBlock value and error if any. @@ -366,28 +371,6 @@ func (c *Contract) DeleteGroupKVUnsigned(ns string, groupID *big.Int, key string return c.actor.MakeUnsignedCall(c.hash, "deleteGroupKV", nil, ns, groupID, key) } -// DeleteOwner creates a transaction invoking `deleteOwner` method of the contract. -// This transaction is signed and immediately sent to the network. -// The values returned are its hash, ValidUntilBlock value and error if any. -func (c *Contract) DeleteOwner(addr util.Uint160) (util.Uint256, uint32, error) { - return c.actor.SendCall(c.hash, "deleteOwner", addr) -} - -// DeleteOwnerTransaction creates a transaction invoking `deleteOwner` method of the contract. -// This transaction is signed, but not sent to the network, instead it's -// returned to the caller. -func (c *Contract) DeleteOwnerTransaction(addr util.Uint160) (*transaction.Transaction, error) { - return c.actor.MakeCall(c.hash, "deleteOwner", addr) -} - -// DeleteOwnerUnsigned creates a transaction invoking `deleteOwner` method of the contract. -// This transaction is not signed, it's simply returned to the caller. -// Any fields of it that do not affect fees can be changed (ValidUntilBlock, -// Nonce), fee values (NetworkFee, SystemFee) can be increased as well. -func (c *Contract) DeleteOwnerUnsigned(addr util.Uint160) (*transaction.Transaction, error) { - return c.actor.MakeUnsignedCall(c.hash, "deleteOwner", nil, addr) -} - // DeleteSubject creates a transaction invoking `deleteSubject` method of the contract. // This transaction is signed and immediately sent to the network. // The values returned are its hash, ValidUntilBlock value and error if any. @@ -718,28 +701,6 @@ func (c *Contract) ListNamespacesUnsigned() (*transaction.Transaction, error) { return c.actor.MakeUnsignedCall(c.hash, "listNamespaces", nil) } -// ListOwners creates a transaction invoking `listOwners` method of the contract. -// This transaction is signed and immediately sent to the network. -// The values returned are its hash, ValidUntilBlock value and error if any. -func (c *Contract) ListOwners() (util.Uint256, uint32, error) { - return c.actor.SendCall(c.hash, "listOwners") -} - -// ListOwnersTransaction creates a transaction invoking `listOwners` method of the contract. -// This transaction is signed, but not sent to the network, instead it's -// returned to the caller. -func (c *Contract) ListOwnersTransaction() (*transaction.Transaction, error) { - return c.actor.MakeCall(c.hash, "listOwners") -} - -// ListOwnersUnsigned creates a transaction invoking `listOwners` method of the contract. -// This transaction is not signed, it's simply returned to the caller. -// Any fields of it that do not affect fees can be changed (ValidUntilBlock, -// Nonce), fee values (NetworkFee, SystemFee) can be increased as well. -func (c *Contract) ListOwnersUnsigned() (*transaction.Transaction, error) { - return c.actor.MakeUnsignedCall(c.hash, "listOwners", nil) -} - // ListSubjects creates a transaction invoking `listSubjects` method of the contract. // This transaction is signed and immediately sent to the network. // The values returned are its hash, ValidUntilBlock value and error if any. @@ -828,6 +789,28 @@ func (c *Contract) RemoveSubjectKeyUnsigned(addr util.Uint160, key *keys.PublicK return c.actor.MakeUnsignedCall(c.hash, "removeSubjectKey", nil, addr, key) } +// SetAdmin creates a transaction invoking `setAdmin` method of the contract. +// This transaction is signed and immediately sent to the network. +// The values returned are its hash, ValidUntilBlock value and error if any. +func (c *Contract) SetAdmin(addr util.Uint160) (util.Uint256, uint32, error) { + return c.actor.SendCall(c.hash, "setAdmin", addr) +} + +// SetAdminTransaction creates a transaction invoking `setAdmin` method of the contract. +// This transaction is signed, but not sent to the network, instead it's +// returned to the caller. +func (c *Contract) SetAdminTransaction(addr util.Uint160) (*transaction.Transaction, error) { + return c.actor.MakeCall(c.hash, "setAdmin", addr) +} + +// SetAdminUnsigned creates a transaction invoking `setAdmin` method of the contract. +// This transaction is not signed, it's simply returned to the caller. +// Any fields of it that do not affect fees can be changed (ValidUntilBlock, +// Nonce), fee values (NetworkFee, SystemFee) can be increased as well. +func (c *Contract) SetAdminUnsigned(addr util.Uint160) (*transaction.Transaction, error) { + return c.actor.MakeUnsignedCall(c.hash, "setAdmin", nil, addr) +} + // SetGroupKV creates a transaction invoking `setGroupKV` method of the contract. // This transaction is signed and immediately sent to the network. // The values returned are its hash, ValidUntilBlock value and error if any. diff --git a/tests/frostfsid_client_test.go b/tests/frostfsid_client_test.go index d6bd9ed..c29c689 100644 --- a/tests/frostfsid_client_test.go +++ b/tests/frostfsid_client_test.go @@ -76,19 +76,19 @@ func TestFrostFSID_Client_ContractOwnersManagement(t *testing.T) { defaultOwnerAddress := ffsid.base.owner.ScriptHash() _, newOwnerAddress := newKey(t) - checkListOwnersClient(t, ffsid.cli, defaultOwnerAddress) + checkAdminClient(t, ffsid.cli, defaultOwnerAddress) - _, _, err := ffsid.cli.AddOwner(newOwnerAddress) + _, _, err := ffsid.cli.SetAdmin(newOwnerAddress) require.ErrorContains(t, err, "not witnessed") - committeeInvoker.Invoke(t, stackitem.Null{}, addOwnerMethod, newOwnerAddress) + committeeInvoker.Invoke(t, stackitem.Null{}, setAdminMethod, newOwnerAddress) - checkListOwnersClient(t, ffsid.cli, defaultOwnerAddress, newOwnerAddress) + checkAdminClient(t, ffsid.cli, newOwnerAddress) - _, _, err = ffsid.cli.DeleteOwner(newOwnerAddress) + _, _, err = ffsid.cli.ClearAdmin() require.ErrorContains(t, err, "not witnessed") - committeeInvoker.Invoke(t, stackitem.Null{}, deleteOwnerMethod, newOwnerAddress) + committeeInvoker.Invoke(t, stackitem.Null{}, clearAdminMethod) - checkListOwnersClient(t, ffsid.cli, defaultOwnerAddress) + checkAdminClient(t, ffsid.cli) } func newKey(t *testing.T) (*keys.PrivateKey, util.Uint160) { @@ -97,10 +97,13 @@ func newKey(t *testing.T) (*keys.PrivateKey, util.Uint160) { return key, key.PublicKey().GetScriptHash() } -func checkListOwnersClient(t *testing.T, cli *client.Client, owners ...util.Uint160) { - addresses, err := cli.ListOwners() +func checkAdminClient(t *testing.T, cli *client.Client, owners ...util.Uint160) { + address, isSet, err := cli.GetAdmin() require.NoError(t, err) - require.ElementsMatch(t, addresses, owners) + require.Equal(t, len(owners) > 0, isSet) + if isSet { + require.Equal(t, owners[0], address) + } } func TestFrostFSID_Client_SubjectManagement(t *testing.T) { diff --git a/tests/frostfsid_test.go b/tests/frostfsid_test.go index 7a7e23d..d6ec2cc 100644 --- a/tests/frostfsid_test.go +++ b/tests/frostfsid_test.go @@ -23,9 +23,9 @@ import ( const frostfsidPath = "../frostfsid" const ( - addOwnerMethod = "addOwner" - deleteOwnerMethod = "deleteOwner" - listOwnersMethod = "listOwners" + setAdminMethod = "setAdmin" + getAdminMethod = "getAdmin" + clearAdminMethod = "clearAdmin" createSubjectMethod = "createSubject" getSubjectMethod = "getSubject" @@ -97,7 +97,7 @@ func newSigner(t *testing.T, c *neotest.ContractInvoker, acc *wallet.Account) ne func deployFrostFSIDContract(t *testing.T, e *neotest.Executor, contractOwner util.Uint160) util.Uint160 { args := make([]any, 5) - args[0] = []any{contractOwner} + args[0] = contractOwner c := neotest.CompileFile(t, e.CommitteeHash, frostfsidPath, path.Join(frostfsidPath, "config.yml")) e.DeployContract(t, c, args) @@ -130,30 +130,48 @@ func TestFrostFSID_ContractOwnersManagement(t *testing.T) { invokerHash := invoker.Signers[0].ScriptHash() committeeInvoker := f.CommitteeInvoker() - checkListOwners(t, anonInvoker, invokerHash) + checkOwner(t, anonInvoker, invokerHash) anonInvoker.InvokeFail(t, notWitnessedError, createNamespaceMethod, "namespace") invoker.Invoke(t, stackitem.Null{}, createNamespaceMethod, "namespace") - invoker.InvokeFail(t, notWitnessedError, addOwnerMethod, anonInvokerHash) - committeeInvoker.Invoke(t, stackitem.Null{}, addOwnerMethod, anonInvokerHash) - anonInvoker.Invoke(t, stackitem.Null{}, createNamespaceMethod, "namespace2") + t.Run("setAdmin is only allowed for committee", func(t *testing.T) { + invoker.InvokeFail(t, notWitnessedError, setAdminMethod, anonInvokerHash) + }) - checkListOwners(t, anonInvoker, invokerHash, anonInvokerHash) + t.Run("replace owner", func(t *testing.T) { + committeeInvoker.Invoke(t, stackitem.Null{}, setAdminMethod, anonInvokerHash) + checkOwner(t, anonInvoker, anonInvokerHash) - anonInvoker.InvokeFail(t, notWitnessedError, deleteOwnerMethod, anonInvokerHash) - committeeInvoker.Invoke(t, stackitem.Null{}, deleteOwnerMethod, anonInvokerHash) - anonInvoker.InvokeFail(t, notWitnessedError, createNamespaceMethod, "namespace3") + invoker.InvokeFail(t, notWitnessedError, createNamespaceMethod, "namespace2") + anonInvoker.Invoke(t, stackitem.Null{}, createNamespaceMethod, "namespace2") + }) + t.Run("remove owner", func(t *testing.T) { + committeeInvoker.Invoke(t, stackitem.Null{}, clearAdminMethod) + checkOwner(t, anonInvoker) - checkListOwners(t, anonInvoker, invokerHash) + invoker.InvokeFail(t, notWitnessedError, createNamespaceMethod, "namespace3") + anonInvoker.InvokeFail(t, notWitnessedError, createNamespaceMethod, "namespace3") + }) } -func checkListOwners(t *testing.T, invoker *neotest.ContractInvoker, expectedAddresses ...util.Uint160) { - s, err := invoker.TestInvoke(t, listOwnersMethod) +func checkOwner(t *testing.T, invoker *neotest.ContractInvoker, owner ...util.Uint160) { + if len(owner) > 1 { + require.Fail(t, "invalid testcase") + } + + s, err := invoker.TestInvoke(t, getAdminMethod) require.NoError(t, err) - addresses, err := unwrap.ArrayOfUint160(makeValidRes(stackitem.NewArray(readIteratorAll(s))), nil) + require.Equal(t, 1, s.Len(), "unexpected number items on stack") + if len(owner) == 0 { + _, isMissing := s.Pop().Item().(stackitem.Null) + require.True(t, isMissing) + return + } + + bs, err := s.Pop().Item().TryBytes() require.NoError(t, err) - require.ElementsMatch(t, addresses, expectedAddresses) + require.Equal(t, bs, owner[0].BytesBE()) } func TestFrostFSID_SubjectManagement(t *testing.T) {