From 50f5852ae42a55e40e3fa43b6ef3ddc6b4471e35 Mon Sep 17 00:00:00 2001 From: vsmk98 Date: Mon, 9 Sep 2019 15:53:15 +0800 Subject: [PATCH] permission: removed unnecessary mux from permissions_cache.go, added additional unit test for permissions cache, code clean up and documentation change --- core/types/permissions_cache.go | 51 ++++++++------------- core/types/permissions_cache_test.go | 11 ++++- docs/Permissioning/setup.md | 4 +- permission/permission.go | 66 +++++----------------------- permission/permission_test.go | 31 +++++++------ 5 files changed, 59 insertions(+), 104 deletions(-) diff --git a/core/types/permissions_cache.go b/core/types/permissions_cache.go index 43ae74a99..c74f1dbf9 100644 --- a/core/types/permissions_cache.go +++ b/core/types/permissions_cache.go @@ -92,21 +92,21 @@ type OrgDetailInfo struct { // permission config for bootstrapping type PermissionConfig struct { - UpgrdAddress common.Address - InterfAddress common.Address - ImplAddress common.Address - NodeAddress common.Address - AccountAddress common.Address - RoleAddress common.Address - VoterAddress common.Address - OrgAddress common.Address - NwAdminOrg string - NwAdminRole string - OrgAdminRole string + UpgrdAddress common.Address `json:"upgrdableAddress"` + InterfAddress common.Address `json:"interfaceAddress"` + ImplAddress common.Address `json:"implAddress"` + NodeAddress common.Address `json:"nodeMgrAddress"` + AccountAddress common.Address `json:"accountMgrAddress"` + RoleAddress common.Address `json:"roleMgrAddress"` + VoterAddress common.Address `json:"voterMgrAddress"` + OrgAddress common.Address `json:"orgMgrAddress"` + NwAdminOrg string `json:"nwAdminOrg"` + NwAdminRole string `json:"nwAdminRole"` + OrgAdminRole string `json:"orgAdminRole"` - Accounts []common.Address //initial list of account that need full access - SubOrgDepth big.Int - SubOrgBreadth big.Int + Accounts []common.Address `json:"accounts"` //initial list of account that need full access + SubOrgDepth *big.Int `json:"subOrgBreadth"` + SubOrgBreadth *big.Int `json:"subOrgDepth"` } type OrgKey struct { @@ -134,17 +134,14 @@ type OrgCache struct { type NodeCache struct { c *lru.Cache - mux sync.Mutex } type RoleCache struct { c *lru.Cache - mux sync.Mutex } type AcctCache struct { c *lru.Cache - mux sync.Mutex } func NewOrgCache() *OrgCache { @@ -154,17 +151,17 @@ func NewOrgCache() *OrgCache { func NewNodeCache() *NodeCache { c, _ := lru.New(defaultMapLimit) - return &NodeCache{c, sync.Mutex{}} + return &NodeCache{c} } func NewRoleCache() *RoleCache { c, _ := lru.New(defaultMapLimit) - return &RoleCache{c, sync.Mutex{}} + return &RoleCache{c} } func NewAcctCache() *AcctCache { c, _ := lru.New(defaultMapLimit) - return &AcctCache{c, sync.Mutex{}} + return &AcctCache{c} } var syncStarted = false @@ -200,7 +197,7 @@ func SetDefaults(nwRoleId, oaRoleId string) { orgAdminRole = oaRoleId } -func GetDefaults() (string, string, AccessType){ +func GetDefaults() (string, string, AccessType) { return networkAdminRole, orgAdminRole, DefaultAccess } @@ -256,15 +253,11 @@ func (o *OrgCache) GetOrgList() []OrgInfo { } func (n *NodeCache) UpsertNode(orgId string, url string, status NodeStatus) { - defer n.mux.Unlock() - n.mux.Lock() key := NodeKey{OrgId: orgId, Url: url} n.c.Add(key, &NodeInfo{orgId, url, status}) } func (n *NodeCache) GetNodeByUrl(url string) *NodeInfo { - defer n.mux.Unlock() - n.mux.Lock() for _, k := range n.c.Keys() { ent := k.(NodeKey) if ent.Url == url { @@ -286,15 +279,11 @@ func (n *NodeCache) GetNodeList() []NodeInfo { } func (a *AcctCache) UpsertAccount(orgId string, role string, acct common.Address, orgAdmin bool, status AcctStatus) { - defer a.mux.Unlock() - a.mux.Lock() key := AccountKey{acct} a.c.Add(key, &AccountInfo{orgId, role, acct, orgAdmin, status}) } func (a *AcctCache) GetAccount(acct common.Address) *AccountInfo { - defer a.mux.Unlock() - a.mux.Lock() if v, ok := a.c.Get(AccountKey{acct}); ok { return v.(*AccountInfo) } @@ -337,16 +326,12 @@ func (a *AcctCache) GetAcctListRole(orgId, roleId string) []AccountInfo { } func (r *RoleCache) UpsertRole(orgId string, role string, voter bool, admin bool, access AccessType, active bool) { - defer r.mux.Unlock() - r.mux.Lock() key := RoleKey{orgId, role} r.c.Add(key, &RoleInfo{orgId, role, voter, admin, access, active}) } func (r *RoleCache) GetRole(orgId string, roleId string) *RoleInfo { - defer r.mux.Unlock() - r.mux.Lock() key := RoleKey{OrgId: orgId, RoleId: roleId} if ent, ok := r.c.Get(key); ok { return ent.(*RoleInfo) diff --git a/core/types/permissions_cache_test.go b/core/types/permissions_cache_test.go index 4cf15505a..4613b7c65 100644 --- a/core/types/permissions_cache_test.go +++ b/core/types/permissions_cache_test.go @@ -59,9 +59,18 @@ func TestOrgCache_UpsertOrg(t *testing.T) { //add another org and check get org list OrgInfoMap.UpsertOrg(ORGADMIN, "", ORGADMIN, big.NewInt(1), OrgApproved) - orgList := OrgInfoMap.GetOrgList() assert.True(len(orgList) == 2, fmt.Sprintf("Expected 2 entries, got %v", len(orgList))) + + //add sub org and check get orglist + OrgInfoMap.UpsertOrg("SUB1", ORGADMIN, ORGADMIN, big.NewInt(2), OrgApproved) + orgList = OrgInfoMap.GetOrgList() + assert.True(len(orgList) == 3, fmt.Sprintf("Expected 3 entries, got %v", len(orgList))) + + //suspend the sub org and check get orglist + OrgInfoMap.UpsertOrg("SUB1", ORGADMIN, ORGADMIN, big.NewInt(2), OrgSuspended) + orgList = OrgInfoMap.GetOrgList() + assert.True(len(orgList) == 3, fmt.Sprintf("Expected 3 entries, got %v", len(orgList))) } func TestNodeCache_UpsertNode(t *testing.T) { diff --git a/docs/Permissioning/setup.md b/docs/Permissioning/setup.md index f3a88ad2f..215b29bfc 100644 --- a/docs/Permissioning/setup.md +++ b/docs/Permissioning/setup.md @@ -21,8 +21,8 @@ The steps to enable new permissions model are as described below: "nwAdminRole" : "ADMIN", "orgAdminRole" : "ORGADMIN", "accounts":["0xed9d02e382b34818e88b88a309c7fe71e65f419d", "0xca843569e3427144cead5e4d5999a3d0ccf92b8e"], - "subOrgBreadth" : "3", - "subOrgDepth" : "4" + "subOrgBreadth" : 3, + "subOrgDepth" : 4 } ``` > * `upgradableAddress` is the address of deployed contract `PermissionsUpgradable.sol` diff --git a/permission/permission.go b/permission/permission.go index 50f8a69b2..18f0c6cdd 100644 --- a/permission/permission.go +++ b/permission/permission.go @@ -20,7 +20,6 @@ import ( "github.com/ethereum/go-ethereum/rpc" "github.com/ethereum/go-ethereum/accounts/abi/bind" - "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/eth" "github.com/ethereum/go-ethereum/log" @@ -38,25 +37,6 @@ const ( NodeDelete ) -// permission config for bootstrapping -type PermissionLocalConfig struct { - UpgrdAddress string `json:"upgrdableAddress"` - InterfAddress string `json:"interfaceAddress"` - ImplAddress string `json:"implAddress"` - NodeAddress string `json:"nodeMgrAddress"` - AccountAddress string `json:"accountMgrAddress"` - RoleAddress string `json:"roleMgrAddress"` - VoterAddress string `json:"voterMgrAddress"` - OrgAddress string `json:"orgMgrAddress"` - NwAdminOrg string `json:"nwAdminOrg"` - NwAdminRole string `json:"nwAdminRole"` - OrgAdminRole string `json:"orgAdminRole"` - - Accounts []string `json:"accounts"` //initial list of account that need full access - SubOrgBreadth string `json:"subOrgBreadth"` - SubOrgDepth string `json:"subOrgDepth"` -} - type PermissionCtrl struct { node *node.Node ethClnt bind.ContractBackend @@ -82,32 +62,6 @@ type PermissionCtrl struct { type stopEvent struct { } -// converts local permissions data to global permissions config -func populateConfig(config PermissionLocalConfig) types.PermissionConfig { - var permConfig types.PermissionConfig - permConfig.UpgrdAddress = common.HexToAddress(config.UpgrdAddress) - permConfig.InterfAddress = common.HexToAddress(config.InterfAddress) - permConfig.ImplAddress = common.HexToAddress(config.ImplAddress) - permConfig.OrgAddress = common.HexToAddress(config.OrgAddress) - permConfig.RoleAddress = common.HexToAddress(config.RoleAddress) - permConfig.NodeAddress = common.HexToAddress(config.NodeAddress) - permConfig.AccountAddress = common.HexToAddress(config.AccountAddress) - permConfig.VoterAddress = common.HexToAddress(config.VoterAddress) - - permConfig.NwAdminOrg = config.NwAdminOrg - permConfig.NwAdminRole = config.NwAdminRole - permConfig.OrgAdminRole = config.OrgAdminRole - - // populate the account list as passed in config - for _, val := range config.Accounts { - permConfig.Accounts = append(permConfig.Accounts, common.HexToAddress(val)) - } - permConfig.SubOrgBreadth.SetString(config.SubOrgBreadth, 10) - permConfig.SubOrgDepth.SetString(config.SubOrgDepth, 10) - - return permConfig -} - // function reads the permissions config file passed and populates the // config structure accordingly func ParsePermissionConfig(dir string) (types.PermissionConfig, error) { @@ -120,24 +74,28 @@ func ParsePermissionConfig(dir string) (types.PermissionConfig, error) { defer func() { _ = f.Close() }() - var permlocConfig PermissionLocalConfig - if err := json.NewDecoder(f).Decode(&permlocConfig); err != nil { - log.Error("error unmarshalling file", "file", fullPath, "error", err) - return types.PermissionConfig{}, err + + var permConfig types.PermissionConfig + blob, err := ioutil.ReadFile(fullPath) + if err != nil { + log.Error("error reading file", "err", err, "file", fullPath) + } + + err = json.Unmarshal(blob, &permConfig) + if err != nil { + log.Error("error unmarshalling the file", "err", err, "file", fullPath) } - permConfig := populateConfig(permlocConfig) if len(permConfig.Accounts) == 0 { return types.PermissionConfig{}, fmt.Errorf("no accounts given in %s. Network cannot boot up", params.PERMISSION_MODEL_CONFIG) } - if permConfig.SubOrgDepth.Cmp(big.NewInt(0)) == 0 || permConfig.SubOrgBreadth.Cmp(big.NewInt(0)) == 0 { return types.PermissionConfig{}, fmt.Errorf("sub org breadth depth not passed in %s. Network cannot boot up", params.PERMISSION_MODEL_CONFIG) } - if permConfig.IsEmpty() { return types.PermissionConfig{}, fmt.Errorf("missing contract addresses in %s", params.PERMISSION_MODEL_CONFIG) } + return permConfig, nil } @@ -672,7 +630,7 @@ func (p *PermissionCtrl) bootupNetwork(permInterfSession *pbind.PermInterfaceSes log.Error("bootupNetwork SetPolicy failed", "err", err) return err } - if _, err := permInterfSession.Init(&p.permConfig.SubOrgBreadth, &p.permConfig.SubOrgDepth); err != nil { + if _, err := permInterfSession.Init(p.permConfig.SubOrgBreadth, p.permConfig.SubOrgDepth); err != nil { log.Error("bootupNetwork init failed", "err", err) return err } diff --git a/permission/permission_test.go b/permission/permission_test.go index 6ab0d0e9f..044bd8b13 100644 --- a/permission/permission_test.go +++ b/permission/permission_test.go @@ -480,8 +480,8 @@ func typicalPermissionCtrl(t *testing.T) *PermissionCtrl { Accounts: []common.Address{ guardianAddress, }, - SubOrgDepth: *big.NewInt(3), - SubOrgBreadth: *big.NewInt(3), + SubOrgDepth: big.NewInt(3), + SubOrgBreadth: big.NewInt(3), }) if err != nil { t.Fatal(err) @@ -572,17 +572,20 @@ func TestParsePermissionConfig(t *testing.T) { assert.True(t, err != nil, "expected unmarshalling error") // write permission-config.json into the temp dir - var tmpPermCofig PermissionLocalConfig + var tmpPermCofig types.PermissionConfig tmpPermCofig.NwAdminOrg = arbitraryNetworkAdminOrg tmpPermCofig.NwAdminRole = arbitraryNetworkAdminRole tmpPermCofig.OrgAdminRole = arbitraryOrgAdminRole - tmpPermCofig.InterfAddress = "0x0" - tmpPermCofig.ImplAddress = "0x0" - tmpPermCofig.UpgrdAddress = "0x0" - tmpPermCofig.VoterAddress = "0x0" - tmpPermCofig.RoleAddress = "0x0" - tmpPermCofig.OrgAddress = "0x0" - tmpPermCofig.NodeAddress = "0x0" + tmpPermCofig.InterfAddress = common.Address{} + tmpPermCofig.ImplAddress = common.Address{} + tmpPermCofig.UpgrdAddress = common.Address{} + tmpPermCofig.VoterAddress = common.Address{} + tmpPermCofig.RoleAddress = common.Address{} + tmpPermCofig.OrgAddress = common.Address{} + tmpPermCofig.NodeAddress = common.Address{} + tmpPermCofig.SubOrgBreadth = new(big.Int) + tmpPermCofig.SubOrgDepth = new(big.Int) + blob, err := json.Marshal(tmpPermCofig) if err := ioutil.WriteFile(fileName, blob, 0644); err != nil { t.Fatal("Error writing new node info to file", "fileName", fileName, "err", err) @@ -591,8 +594,8 @@ func TestParsePermissionConfig(t *testing.T) { assert.True(t, err != nil, "expected sub org depth not set error") _ = os.Remove(fileName) - tmpPermCofig.SubOrgBreadth = "4" - tmpPermCofig.SubOrgDepth = "4" + tmpPermCofig.SubOrgBreadth.Set(big.NewInt(4)) + tmpPermCofig.SubOrgDepth.Set(big.NewInt(4)) blob, _ = json.Marshal(tmpPermCofig) if err := ioutil.WriteFile(fileName, blob, 0644); err != nil { t.Fatal("Error writing new node info to file", "fileName", fileName, "err", err) @@ -601,7 +604,7 @@ func TestParsePermissionConfig(t *testing.T) { assert.True(t, err != nil, "expected account not given error") _ = os.Remove(fileName) - tmpPermCofig.Accounts = append(tmpPermCofig.Accounts, "0xed9d02e382b34818e88b88a309c7fe71e65f419d") + tmpPermCofig.Accounts = append(tmpPermCofig.Accounts, common.StringToAddress("0xed9d02e382b34818e88b88a309c7fe71e65f419d")) blob, err = json.Marshal(tmpPermCofig) if err := ioutil.WriteFile(fileName, blob, 0644); err != nil { t.Fatal("Error writing new node info to file", "fileName", fileName, "err", err) @@ -610,7 +613,7 @@ func TestParsePermissionConfig(t *testing.T) { assert.True(t, err != nil, "expected contract address error") _ = os.Remove(fileName) - tmpPermCofig.InterfAddress = "0xed9d02e382b34818e88b88a309c7fe71e65f419d" + tmpPermCofig.InterfAddress = common.StringToAddress("0xed9d02e382b34818e88b88a309c7fe71e65f419d") blob, err = json.Marshal(tmpPermCofig) if err := ioutil.WriteFile(fileName, blob, 0644); err != nil { t.Fatal("Error writing new node info to file", "fileName", fileName, "err", err)