aarifullin/neoneo-go
1
0
Fork 0
forked from TrueCloudLab/neoneo-go
Code Pull requests Activity

Merge pull request #1170 from nspcc-dev/neo3/mempool/verify_fix

Browse source 
core: prevent concurrent map writes during (*mp).Verify
This commit is contained in:
Roman Khimov 2020-07-09 22:08:50 +03:00 committed by GitHub
commit ace877ab68
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 20 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

40
pkg/core/mempool/mem_pool.go
View file

@ -107,35 +107,26 @@ func (mp *Pool) containsKey(hash util.Uint256) bool {
} }
// tryAddSendersFee tries to add system fee and network fee to the total sender`s fee in mempool // tryAddSendersFee tries to add system fee and network fee to the total sender`s fee in mempool
// and returns false if sender has not enough GAS to pay // and returns false if both balance check is required and sender has not enough GAS to pay
func (mp *Pool) tryAddSendersFee(tx *transaction.Transaction, feer Feer) bool { func (mp *Pool) tryAddSendersFee(tx *transaction.Transaction, feer Feer, needCheck bool) bool {
if !mp.checkBalanceAndUpdate(tx, feer) {
return false
}
mp.addSendersFee(tx)
return true
}
// checkBalanceAndUpdate returns true in case when sender has enough GAS to pay for
// the transaction and sets sender's balance value in mempool in case if it was not set
func (mp *Pool) checkBalanceAndUpdate(tx *transaction.Transaction, feer Feer) bool {
senderFee, ok := mp.fees[tx.Sender] senderFee, ok := mp.fees[tx.Sender]
if !ok { if !ok {
senderFee.balance = feer.GetUtilityTokenBalance(tx.Sender) senderFee.balance = feer.GetUtilityTokenBalance(tx.Sender)
mp.fees[tx.Sender] = senderFee mp.fees[tx.Sender] = senderFee
} }
needFee := senderFee.feeSum + tx.SystemFee + tx.NetworkFee if needCheck && !checkBalance(tx, senderFee) {
if senderFee.balance.Cmp(big.NewInt(needFee)) < 0 {
return false return false
} }
senderFee.feeSum += tx.SystemFee + tx.NetworkFee
mp.fees[tx.Sender] = senderFee
return true return true
} }
// addSendersFee adds system fee and network fee to the total sender`s fee in mempool // checkBalance returns true in case when sender has enough GAS to pay for the
func (mp *Pool) addSendersFee(tx *transaction.Transaction) { // transaction
senderFee := mp.fees[tx.Sender] func checkBalance(tx *transaction.Transaction, balance utilityBalanceAndFees) bool {
senderFee.feeSum += tx.SystemFee + tx.NetworkFee needFee := balance.feeSum + tx.SystemFee + tx.NetworkFee
mp.fees[tx.Sender] = senderFee return balance.balance.Cmp(big.NewInt(needFee)) >= 0
} }
// Add tries to add given transaction to the Pool. // Add tries to add given transaction to the Pool.
@ -183,7 +174,8 @@ func (mp *Pool) Add(t *transaction.Transaction, fee Feer) error {
copy(mp.verifiedTxes[n+1:], mp.verifiedTxes[n:]) copy(mp.verifiedTxes[n+1:], mp.verifiedTxes[n:])
mp.verifiedTxes[n] = pItem mp.verifiedTxes[n] = pItem
} }
mp.addSendersFee(pItem.txn) // we already checked balance in checkTxConflicts, so don't need to check again
mp.tryAddSendersFee(pItem.txn, fee, false)
updateMempoolMetrics(len(mp.verifiedTxes)) updateMempoolMetrics(len(mp.verifiedTxes))
mp.lock.Unlock() mp.lock.Unlock()
@ -226,7 +218,7 @@ func (mp *Pool) RemoveStale(isOK func(*transaction.Transaction) bool, feer Feer)
newVerifiedTxes := mp.verifiedTxes[:0] newVerifiedTxes := mp.verifiedTxes[:0]
mp.fees = make(map[util.Uint160]utilityBalanceAndFees) // it'd be nice to reuse existing map, but we can't easily clear it mp.fees = make(map[util.Uint160]utilityBalanceAndFees) // it'd be nice to reuse existing map, but we can't easily clear it
for _, itm := range mp.verifiedTxes { for _, itm := range mp.verifiedTxes {
if isOK(itm.txn) && mp.checkPolicy(itm.txn, policyChanged) && mp.tryAddSendersFee(itm.txn, feer) { if isOK(itm.txn) && mp.checkPolicy(itm.txn, policyChanged) && mp.tryAddSendersFee(itm.txn, feer, true) {
newVerifiedTxes = append(newVerifiedTxes, itm) newVerifiedTxes = append(newVerifiedTxes, itm)
} else { } else {
delete(mp.verifiedMap, itm.txn.Hash()) delete(mp.verifiedMap, itm.txn.Hash())
@ -292,7 +284,11 @@ func (mp *Pool) GetVerifiedTransactions() []*transaction.Transaction {
// checkTxConflicts is an internal unprotected version of Verify. // checkTxConflicts is an internal unprotected version of Verify.
func (mp *Pool) checkTxConflicts(tx *transaction.Transaction, fee Feer) bool { func (mp *Pool) checkTxConflicts(tx *transaction.Transaction, fee Feer) bool {
return mp.checkBalanceAndUpdate(tx, fee) senderFee, ok := mp.fees[tx.Sender]
if !ok {
senderFee.balance = fee.GetUtilityTokenBalance(tx.Sender)
}
return checkBalance(tx, senderFee)
} }
// Verify checks if a Sender of tx is able to pay for it (and all the other // Verify checks if a Sender of tx is able to pay for it (and all the other

8
pkg/core/mempool/mem_pool_test.go
View file

@ -187,14 +187,10 @@ func TestMemPoolFees(t *testing.T) {
tx0 := transaction.New(netmode.UnitTestNet, []byte{byte(opcode.PUSH1)}, 0) tx0 := transaction.New(netmode.UnitTestNet, []byte{byte(opcode.PUSH1)}, 0)
tx0.NetworkFee = balance.Int64() + 1 tx0.NetworkFee = balance.Int64() + 1
tx0.Sender = sender0 tx0.Sender = sender0
// insufficient funds to add transaction, but balance should be stored // insufficient funds to add transaction, and balance shouldn't be stored
require.Equal(t, false, mp.Verify(tx0, &FeerStub{})) require.Equal(t, false, mp.Verify(tx0, &FeerStub{}))
require.Error(t, mp.Add(tx0, &FeerStub{})) require.Error(t, mp.Add(tx0, &FeerStub{}))
require.Equal(t, 1, len(mp.fees)) require.Equal(t, 0, len(mp.fees))
require.Equal(t, utilityBalanceAndFees{
balance: balance,
feeSum: 0,
}, mp.fees[sender0])
balancePart := new(big.Int).Div(balance, big.NewInt(4)) balancePart := new(big.Int).Div(balance, big.NewInt(4))
// no problems with adding another transaction with lower fee // no problems with adding another transaction with lower fee