[core + node] Change blacklist map and err returns

* Blacklist map now have values of empty structs
* Err causes are now returned when adding txs to tx-pool

core/tx_pool.go

@@ -86,10 +86,10 @@ var (
 	ErrKnownTransaction = errors.New("known transaction")
 
 	// ErrBlacklistFrom is returned if a transaction's from/source address is blacklisted
-	ErrBlacklistFrom = errors.New("`from` address of transaction is blacklisted")
+	ErrBlacklistFrom = errors.New("`from` address of transaction in blacklist")
 
 	// ErrBlacklistTo is returned if a transaction's to/destination address is blacklisted
-	ErrBlacklistTo = errors.New("`to` address of transaction is blacklisted")
+	ErrBlacklistTo = errors.New("`to` address of transaction in blacklist")
 )
 
 var (
@@ -153,7 +153,7 @@ type TxPoolConfig struct {
 
 	Lifetime time.Duration // Maximum amount of time non-executable transaction are queued
 
-	Blacklist *map[common.Address]bool // Set of accounts that cannot be a part of any transaction
+	Blacklist *map[common.Address]struct{} // Set of accounts that cannot be a part of any transaction
 }
 
 // DefaultTxPoolConfig contains the default configurations for the transaction
@@ -172,7 +172,7 @@ var DefaultTxPoolConfig = TxPoolConfig{
 
 	Lifetime: 3 * time.Hour,
 
-	Blacklist: &map[common.Address]bool{},
+	Blacklist: &map[common.Address]struct{}{},
 }
 
 // sanitize checks the provided user configurations and changes anything that's
@@ -644,17 +644,20 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error {
 		return ErrInvalidSender
 	}
 	// Make sure transaction does not have blacklisted addresses
-	if (*pool.config.Blacklist)[from] {
+	if _, exists := (*pool.config.Blacklist)[from]; exists {
 		if b32, err := hmyCommon.AddressToBech32(from); err == nil {
 			return errors.WithMessagef(ErrBlacklistFrom, "transaction sender is %s", b32)
 		}
 		return ErrBlacklistFrom
 	}
-	if tx.To() != nil && (*pool.config.Blacklist)[*tx.To()] {
+	// Make sure transaction does not burn funds by sending funds to blacklisted address
-		if b32, err := hmyCommon.AddressToBech32(*tx.To()); err == nil {
+	if tx.To() != nil {
-			return errors.WithMessagef(ErrBlacklistTo, "transaction receiver is %s", b32)
+		if _, exists := (*pool.config.Blacklist)[*tx.To()]; exists {
+			if b32, err := hmyCommon.AddressToBech32(*tx.To()); err == nil {
+				return errors.WithMessagef(ErrBlacklistTo, "transaction receiver is %s", b32)
+			}
+			return ErrBlacklistTo
 		}
-		return ErrBlacklistTo
 	}
 	// Drop non-local transactions under our own minimal accepted gas price
 	local = local || pool.locals.contains(from) // account may be local even if the transaction arrived from the network
@@ -871,14 +874,14 @@ func (pool *TxPool) promoteTx(addr common.Address, hash common.Hash, tx *types.T
 // the sender as a local one in the mean time, ensuring it goes around the local
 // pricing constraints.
 func (pool *TxPool) AddLocal(tx *types.Transaction) error {
-	return errors.Cause(pool.addTx(tx, !pool.config.NoLocals))
+	return pool.addTx(tx, !pool.config.NoLocals)
 }
 
 // AddRemote enqueues a single transaction into the pool if it is valid. If the
 // sender is not among the locally tracked ones, full pricing constraints will
 // apply.
 func (pool *TxPool) AddRemote(tx *types.Transaction) error {
-	return errors.Cause(pool.addTx(tx, false))
+	return pool.addTx(tx, false)
 }
 
 // AddLocals enqueues a batch of transactions into the pool if they are valid,
@@ -903,10 +906,11 @@ func (pool *TxPool) addTx(tx *types.Transaction, local bool) error {
 	// Try to inject the transaction and update any state
 	replace, err := pool.add(tx, local)
 	if err != nil {
-		if errors.Cause(err) != ErrKnownTransaction {
+		errCause := errors.Cause(err)
+		if errCause != ErrKnownTransaction {
 			pool.txnErrorSink([]types.RPCTransactionError{*types.NewRPCTransactionError(tx.Hash(), err)})
 		}
-		return err
+		return errCause
 	}
 	// If we added a new transaction, run promotion checks and return
 	if !replace {

core/tx_pool_test.go

@@ -262,7 +262,7 @@ func TestInvalidTransactions(t *testing.T) {
 }
 
 func TestBlacklistedTransactions(t *testing.T) {
-	// DO NOT parallelize, test will add accounts to tx pool config used by other tests.
+	// DO NOT parallelize, test will add accounts to tx pool config.
 
 	// Create the pool
 	pool, _ := setupTxPool()
@@ -283,15 +283,14 @@ func TestBlacklistedTransactions(t *testing.T) {
 	pool.currentState.AddBalance(bannedFromAcc, big.NewInt(50100))
 	pool.currentState.AddBalance(goodFromAcc, big.NewInt(50100))
 
-	(*DefaultTxPoolConfig.Blacklist)[bannedFromAcc] = false
+	(*DefaultTxPoolConfig.Blacklist)[bannedToAcc] = struct{}{}
-	(*DefaultTxPoolConfig.Blacklist)[bannedToAcc] = true
 	err := pool.AddRemotes([]*types.Transaction{badTx})
 	if err[0] != ErrBlacklistTo {
 		t.Error("expected", ErrBlacklistTo, "got", err[0])
 	}
 
-	(*DefaultTxPoolConfig.Blacklist)[bannedFromAcc] = true
+	delete(*DefaultTxPoolConfig.Blacklist, bannedToAcc)
-	(*DefaultTxPoolConfig.Blacklist)[bannedToAcc] = false
+	(*DefaultTxPoolConfig.Blacklist)[bannedFromAcc] = struct{}{}
 	err = pool.AddRemotes([]*types.Transaction{badTx})
 	if err[0] != ErrBlacklistFrom {
 		t.Error("expected", ErrBlacklistFrom, "got", err[0])
@@ -304,7 +303,7 @@ func TestBlacklistedTransactions(t *testing.T) {
 	}
 
 	// cleanup blacklist config for other tests
-	DefaultTxPoolConfig.Blacklist = &map[common.Address]bool{}
+	DefaultTxPoolConfig.Blacklist = &map[common.Address]struct{}{}
 }
 
 func TestTransactionQueue(t *testing.T) {

node/node.go

@@ -449,7 +449,7 @@ func (node *Node) GetSyncID() [SyncIDLength]byte {
 
 // New creates a new node.
 func New(host p2p.Host, consensusObj *consensus.Consensus,
-	chainDBFactory shardchain.DBFactory, blacklist *map[common.Address]bool, isArchival bool) *Node {
+	chainDBFactory shardchain.DBFactory, blacklist *map[common.Address]struct{}, isArchival bool) *Node {
 	node := Node{}
 	const sinkSize = 4096
 	node.errorSink = struct {