Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: improved func ValidateNodeForTxn node id parsing #1578

Merged
merged 3 commits into from
Dec 4, 2022
Merged

perf: improved func ValidateNodeForTxn node id parsing #1578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0c1b78a
perf: improved func ValidateNodeForTxn node id parsing
denis-gudim Dec 1, 2022
39fb519
perf ValidateForTxn benchmark and fixes
denis-gudim Dec 3, 2022
96189e9
Update eth/api_backend.go
denis-gudim Dec 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions eth/api_backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"github.com/ethereum/go-ethereum/event"
"github.com/ethereum/go-ethereum/internal/ethapi"
"github.com/ethereum/go-ethereum/miner"
"github.com/ethereum/go-ethereum/p2p/enode"
"github.com/ethereum/go-ethereum/params"
pcore "github.com/ethereum/go-ethereum/permission/core"
"github.com/ethereum/go-ethereum/rpc"
Expand All @@ -55,8 +56,8 @@ type EthAPIBackend struct {

// Quorum
//
// hex node id from node public key
hexNodeId string
// node id from node public key
nodeId enode.ID

// timeout value for call
evmCallTimeOut time.Duration
Expand Down Expand Up @@ -337,11 +338,13 @@ func (b *EthAPIBackend) SubscribeLogsEvent(ch chan<- []*types.Log) event.Subscri
}

func (b *EthAPIBackend) SendTx(ctx context.Context, signedTx *types.Transaction) error {
// Quourum
// validation for node need to happen here and cannot be done as a part of
// validateTx in tx_pool.go as tx_pool validation will happen in every node
if b.hexNodeId != "" && !pcore.ValidateNodeForTxn(b.hexNodeId, signedTx.From()) {
if !pcore.ValidateNodeForTxn(b.nodeId, signedTx.From()) {
return errors.New("cannot send transaction from this node")
}
// End Quorum
return b.eth.txPool.AddLocal(signedTx)
}

Expand Down
12 changes: 9 additions & 3 deletions eth/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,13 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) {
eth.miner = miner.New(eth, &config.Miner, chainConfig, eth.EventMux(), eth.engine, eth.isLocalBlock)
eth.miner.SetExtra(makeExtraData(config.Miner.ExtraData, eth.blockchain.Config().IsQuorum))

hexNodeId := fmt.Sprintf("%x", crypto.FromECDSAPub(&stack.GetNodeKey().PublicKey)[1:]) // Quorum
// Quorum
hexNodeId := fmt.Sprintf("%x", crypto.FromECDSAPub(&stack.GetNodeKey().PublicKey)[1:])
node, err := enode.ParseV4(hexNodeId)
if err != nil {
return nil, err
}
// End Quorum
if eth.config.QuorumLightClient.Enabled() {
var (
proxyClient *rpc.Client
Expand Down Expand Up @@ -433,9 +439,9 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) {
if ok {
rpcClientSetter.SetRPCClient(proxyClient)
}
eth.APIBackend = &EthAPIBackend{stack.Config().ExtRPCEnabled(), stack.Config().AllowUnprotectedTxs, eth, nil, hexNodeId, config.EVMCallTimeOut, proxyClient}
eth.APIBackend = &EthAPIBackend{stack.Config().ExtRPCEnabled(), stack.Config().AllowUnprotectedTxs, eth, nil, node.ID(), config.EVMCallTimeOut, proxyClient}
} else {
eth.APIBackend = &EthAPIBackend{stack.Config().ExtRPCEnabled(), stack.Config().AllowUnprotectedTxs, eth, nil, hexNodeId, config.EVMCallTimeOut, nil}
eth.APIBackend = &EthAPIBackend{stack.Config().ExtRPCEnabled(), stack.Config().AllowUnprotectedTxs, eth, nil, node.ID(), config.EVMCallTimeOut, nil}
}

if eth.APIBackend.allowUnprotectedTxs {
Expand Down
52 changes: 33 additions & 19 deletions permission/core/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"math/big"
"strings"
"sync"
"sync/atomic"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/p2p/enode"
Expand Down Expand Up @@ -91,6 +92,19 @@ type NodeInfo struct {
OrgId string `json:"orgId"`
Url string `json:"url"`
Status NodeStatus `json:"status"`
id atomic.Value
}

func (n *NodeInfo) ID() enode.ID {
if id := n.id.Load(); id != nil {
return id.(enode.ID)
}
if node, err := enode.ParseV4(n.Url); err == nil {
baptiste-b-pegasys marked this conversation as resolved.
Show resolved Hide resolved
id := node.ID()
n.id.Store(id)
return id
}
return enode.ID{}
}

type RoleInfo struct {
Expand Down Expand Up @@ -118,7 +132,6 @@ type OrgDetailInfo struct {
}

var syncStarted = false

var defaultAccess = FullAccess
var qip714BlockReached = false
var networkBootUpCompleted = false
Expand Down Expand Up @@ -369,7 +382,7 @@ func (o *OrgCache) GetOrgList() []OrgInfo {

func (n *NodeCache) UpsertNode(orgId string, url string, status NodeStatus) {
key := NodeKey{OrgId: orgId, Url: url}
n.c.Add(key, &NodeInfo{orgId, url, status})
n.c.Add(key, &NodeInfo{OrgId: orgId, Url: url, Status: status})
}

func (n *NodeCache) GetNodeByUrl(url string) (*NodeInfo, error) {
Expand Down Expand Up @@ -398,12 +411,19 @@ func (n *NodeCache) GetNodeByUrl(url string) (*NodeInfo, error) {
return nil, errors.New("Node does not exist")
}

func (n *NodeCache) GetNodeList() []NodeInfo {
olist := make([]NodeInfo, len(n.c.Keys()))
func (n *NodeCache) getSourceList() []*NodeInfo {
olist := make([]*NodeInfo, len(n.c.Keys()))
for i, k := range n.c.Keys() {
v, _ := n.c.Get(k)
vp := v.(*NodeInfo)
olist[i] = *vp
olist[i] = v.(*NodeInfo)
}
return olist
}

func (n *NodeCache) GetNodeList() []NodeInfo {
olist := make([]NodeInfo, len(n.c.Keys()))
for i, v := range n.getSourceList() {
olist[i] = *v
}
return olist
}
Expand Down Expand Up @@ -538,7 +558,7 @@ func GetAcctAccess(acctId common.Address) AccessType {
return defaultAccess
}

//checks if the given org is active in the network
// checks if the given org is active in the network
func checkIfOrgActive(orgId string) bool {
o, _ := OrgInfoMap.GetOrg(orgId)
if o != nil && o.Status != OrgSuspended {
Expand Down Expand Up @@ -579,16 +599,11 @@ func CheckIfAdminAccount(acctId common.Address) bool {
}

// validates if the account can transact from the current node
func ValidateNodeForTxn(hexnodeId string, from common.Address) bool {
if !PermissionsEnabled() || hexnodeId == "" {
func ValidateNodeForTxn(nodeId enode.ID, from common.Address) bool {
if !PermissionsEnabled() || nodeId == (enode.ID{}) {
return true
}

passedEnodeId, err := enode.ParseV4(hexnodeId)
if err != nil {
return false
}

ac, _ := AcctInfoMap.GetAccount(from)
if ac == nil {
return true
Expand All @@ -600,20 +615,19 @@ func ValidateNodeForTxn(hexnodeId string, from common.Address) bool {
}

// scan through the node list and validate
for _, n := range NodeInfoMap.GetNodeList() {
for _, n := range NodeInfoMap.getSourceList() {
orgRec, err := OrgInfoMap.GetOrg(n.OrgId)
if err != nil {
return false
}
if orgRec.UltimateParent == acOrgRec.UltimateParent {
recEnodeId, _ := enode.ParseV4(n.Url)
if recEnodeId.ID() == passedEnodeId.ID() && n.Status == NodeApproved {
if n.ID() == nodeId && n.Status == NodeApproved {
return true
}
}
}
if NodeInfoMap.evicted {
return NodeInfoMap.populateAndValidateFunc(hexnodeId, acOrgRec.UltimateParent)
return NodeInfoMap.populateAndValidateFunc(nodeId.String(), acOrgRec.UltimateParent)
}

return false
Expand All @@ -623,7 +637,7 @@ func IsV2Permission() bool {
return PermissionModel == V2
}

// checks if the account permission allows the transaction to be executed
// checks if the account permission allows the transaction to be executed
func IsTransactionAllowed(from common.Address, to common.Address, value *big.Int, gasPrice *big.Int, gasLimit *big.Int, payload []byte, transactionType TransactionType) error {
//if we have not reached QIP714 block return full access
if !PermissionsEnabled() {
Expand Down
38 changes: 32 additions & 6 deletions permission/core/cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/p2p/enode"
"github.com/ethereum/go-ethereum/params"
testifyassert "github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -209,35 +210,59 @@ func TestGetAcctAccess(t *testing.T) {
assert.True(access == ReadOnly, fmt.Sprintf("Expected account access to be %v, got %v", ReadOnly, access))
}

func BenchmarkValidateNodeForTxn(b *testing.B) {
SetQIP714BlockReached()
SetNetworkBootUpCompleted()

OrgInfoMap = NewOrgCache(params.DEFAULT_ORGCACHE_SIZE)
NodeInfoMap = NewNodeCache(params.DEFAULT_NODECACHE_SIZE)
AcctInfoMap = NewAcctCache(params.DEFAULT_ACCOUNTCACHE_SIZE)

// populate an org, account and node. validate access
OrgInfoMap.UpsertOrg(NETWORKADMIN, "", NETWORKADMIN, big.NewInt(1), OrgApproved)
NodeInfoMap.UpsertNode(NETWORKADMIN, NODE1, NodeApproved)
AcctInfoMap.UpsertAccount(NETWORKADMIN, NETWORKADMIN, Acct1, true, AcctActive)
node1, _ := enode.ParseV4(NODE1)

b.ResetTimer()

b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
ValidateNodeForTxn(node1.ID(), Acct1)
}
})
}

func TestValidateNodeForTxn(t *testing.T) {
assert := testifyassert.New(t)
// pass the enode as null and the response should be true
txnAllowed := ValidateNodeForTxn("", Acct1)
txnAllowed := ValidateNodeForTxn(enode.ID{}, Acct1)
assert.True(txnAllowed, "Expected access %v, got %v", true, txnAllowed)

SetQIP714BlockReached()
SetNetworkBootUpCompleted()
// if a proper enode id is not passed, return should be false
txnAllowed = ValidateNodeForTxn("ABCDE", Acct1)
txnAllowed = ValidateNodeForTxn(enode.ID{0x41, 0x43, 0x43, 0x44, 0x45}, Acct1)
assert.True(!txnAllowed, "Expected access %v, got %v", true, txnAllowed)

// if cache is not populated but the enode and account details are proper,
// should return true
txnAllowed = ValidateNodeForTxn(NODE1, Acct1)
node1, _ := enode.ParseV4(NODE1)
txnAllowed = ValidateNodeForTxn(node1.ID(), Acct1)
assert.True(txnAllowed, "Expected access %v, got %v", true, txnAllowed)

// populate an org, account and node. validate access
OrgInfoMap.UpsertOrg(NETWORKADMIN, "", NETWORKADMIN, big.NewInt(1), OrgApproved)
NodeInfoMap.UpsertNode(NETWORKADMIN, NODE1, NodeApproved)
AcctInfoMap.UpsertAccount(NETWORKADMIN, NETWORKADMIN, Acct1, true, AcctActive)
txnAllowed = ValidateNodeForTxn(NODE1, Acct1)
txnAllowed = ValidateNodeForTxn(node1.ID(), Acct1)
assert.True(txnAllowed, "Expected access %v, got %v", true, txnAllowed)

// test access from a node not linked to the org. should return false
OrgInfoMap.UpsertOrg(ORGADMIN, "", ORGADMIN, big.NewInt(1), OrgApproved)
NodeInfoMap.UpsertNode(ORGADMIN, NODE2, NodeApproved)
AcctInfoMap.UpsertAccount(ORGADMIN, ORGADMIN, Acct2, true, AcctActive)
txnAllowed = ValidateNodeForTxn(NODE1, Acct2)
txnAllowed = ValidateNodeForTxn(node1.ID(), Acct2)
assert.True(!txnAllowed, "Expected access %v, got %v", true, txnAllowed)
}

Expand All @@ -248,11 +273,12 @@ func TestValidateNodeForTxn_whenUsingOnlyHexNodeId(t *testing.T) {
AcctInfoMap.UpsertAccount(NETWORKADMIN, NETWORKADMIN, Acct1, true, AcctActive)
arbitraryPrivateKey, _ := crypto.GenerateKey()
hexNodeId := fmt.Sprintf("%x", crypto.FromECDSAPub(&arbitraryPrivateKey.PublicKey)[1:])
node, _ := enode.ParseV4(hexNodeId)

SetQIP714BlockReached()
SetNetworkBootUpCompleted()

txnAllowed := ValidateNodeForTxn(hexNodeId, Acct1)
txnAllowed := ValidateNodeForTxn(node.ID(), Acct1)

testifyassert.False(t, txnAllowed)
}
Expand Down