Skip to content

Commit

Permalink
Merge pull request #3061 from nspcc-dev/check-onchain-conflicts
Browse files Browse the repository at this point in the history 
core: check the signers of on-chained conflicting transaction during new transaction verification
  • Loading branch information
@roman-khimov
roman-khimov authored Jul 21, 2023
2 parents 7a7d0a0 + ee4b8f8 commit 081f9d3
Show file tree
Hide file tree
Showing 7 changed files with 431 additions and 72 deletions.
19 changes: 6 additions & 13 deletions pkg/core/blockchain.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import (

// Tuning parameters.
const (
version = "0.2.8"
version = "0.2.9"

defaultInitialGAS = 52000000_00000000
defaultGCPeriod = 10000
Expand Down Expand Up @@ -2174,15 +2174,6 @@ func (bc *Blockchain) GetHeader(hash util.Uint256) (*block.Header, error) {
return &block.Header, nil
}

// HasTransaction returns true if the blockchain contains he given
// transaction hash.
func (bc *Blockchain) HasTransaction(hash util.Uint256) bool {
if bc.memPool.ContainsKey(hash) {
return true
}
return errors.Is(bc.dao.HasTransaction(hash), dao.ErrAlreadyExists)
}

// HasBlock returns true if the blockchain contains the given
// block hash.
func (bc *Blockchain) HasBlock(hash util.Uint256) bool {
Expand Down Expand Up @@ -2486,7 +2477,7 @@ func (bc *Blockchain) verifyAndPoolTx(t *transaction.Transaction, pool *mempool.
return fmt.Errorf("%w: net fee is %v, need %v", ErrTxSmallNetworkFee, t.NetworkFee, needNetworkFee)
}
// check that current tx wasn't included in the conflicts attributes of some other transaction which is already in the chain
if err := bc.dao.HasTransaction(t.Hash()); err != nil {
if err := bc.dao.HasTransaction(t.Hash(), t.Signers); err != nil {
switch {
case errors.Is(err, dao.ErrAlreadyExists):
return fmt.Errorf("blockchain: %w", ErrAlreadyExists)
Expand Down Expand Up @@ -2587,7 +2578,9 @@ func (bc *Blockchain) verifyTxAttributes(d *dao.Simple, tx *transaction.Transact
return fmt.Errorf("%w: Conflicts attribute was found, but P2PSigExtensions are disabled", ErrInvalidAttribute)
}
conflicts := tx.Attributes[i].Value.(*transaction.Conflicts)
if err := bc.dao.HasTransaction(conflicts.Hash); errors.Is(err, dao.ErrAlreadyExists) {
// Only fully-qualified dao.ErrAlreadyExists error bothers us here, thus, we
// can safely omit the payer argument to HasTransaction call to improve performance a bit.
if err := bc.dao.HasTransaction(conflicts.Hash, nil); errors.Is(err, dao.ErrAlreadyExists) {
return fmt.Errorf("%w: conflicting transaction %s is already on chain", ErrInvalidAttribute, conflicts.Hash.StringLE())
}
case transaction.NotaryAssistedT:
Expand Down Expand Up @@ -2620,7 +2613,7 @@ func (bc *Blockchain) IsTxStillRelevant(t *transaction.Transaction, txpool *memp
return false
}
if txpool == nil {
if bc.dao.HasTransaction(t.Hash()) != nil {
if bc.dao.HasTransaction(t.Hash(), t.Signers) != nil {
return false
}
} else if txpool.HasConflicts(t, bc) {
Expand Down
286 changes: 265 additions & 21 deletions pkg/core/blockchain_neotest_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1634,28 +1634,272 @@ func TestBlockchain_VerifyTx(t *testing.T) {
})
t.Run("enabled", func(t *testing.T) {
t.Run("dummy on-chain conflict", func(t *testing.T) {
tx := newTestTx(t, h, testScript)
require.NoError(t, accs[0].SignTx(netmode.UnitTestNet, tx))
conflicting := transaction.New([]byte{byte(opcode.RET)}, 1000_0000)
conflicting.ValidUntilBlock = bc.BlockHeight() + 1
conflicting.Signers = []transaction.Signer{
{
Account: validator.ScriptHash(),
Scopes: transaction.CalledByEntry,
},
}
conflicting.Attributes = []transaction.Attribute{
{
Type: transaction.ConflictsT,
Value: &transaction.Conflicts{
Hash: tx.Hash(),
t.Run("on-chain conflict signed by malicious party", func(t *testing.T) {
tx := newTestTx(t, h, testScript)
require.NoError(t, accs[0].SignTx(netmode.UnitTestNet, tx))
conflicting := transaction.New([]byte{byte(opcode.RET)}, 1000_0000)
conflicting.ValidUntilBlock = bc.BlockHeight() + 1
conflicting.Signers = []transaction.Signer{
{
Account: validator.ScriptHash(),
Scopes: transaction.CalledByEntry,
},
},
}
conflicting.NetworkFee = 1000_0000
require.NoError(t, validator.SignTx(netmode.UnitTestNet, conflicting))
e.AddNewBlock(t, conflicting)
require.ErrorIs(t, bc.VerifyTx(tx), core.ErrHasConflicts)
}
conflicting.Attributes = []transaction.Attribute{
{
Type: transaction.ConflictsT,
Value: &transaction.Conflicts{
Hash: tx.Hash(),
},
},
}
conflicting.NetworkFee = 1000_0000
require.NoError(t, validator.SignTx(netmode.UnitTestNet, conflicting))
e.AddNewBlock(t, conflicting)
// We expect `tx` to pass verification, because on-chained `conflicting` doesn't have
// `tx`'s payer in the signers list, thus, `conflicting` should be considered as
// malicious conflict.
require.NoError(t, bc.VerifyTx(tx))
})
t.Run("multiple on-chain conflicts signed by malicious parties", func(t *testing.T) {
m1 := e.NewAccount(t)
m2 := e.NewAccount(t)
m3 := e.NewAccount(t)
good := e.NewAccount(t)

// txGood doesn't conflict with anyone and signed by good signer.
txGood := newTestTx(t, good.ScriptHash(), testScript)
require.NoError(t, good.SignTx(netmode.UnitTestNet, txGood))

// txM1 conflicts with txGood and signed by two malicious signers.
txM1 := newTestTx(t, m1.ScriptHash(), testScript)
txM1.Signers = append(txM1.Signers, transaction.Signer{Account: m2.ScriptHash()})
txM1.Attributes = []transaction.Attribute{
{
Type: transaction.ConflictsT,
Value: &transaction.Conflicts{
Hash: txGood.Hash(),
},
},
}
txM1.NetworkFee = 1_000_0000
require.NoError(t, m1.SignTx(netmode.UnitTestNet, txM1))
require.NoError(t, m2.SignTx(netmode.UnitTestNet, txM1))
e.AddNewBlock(t, txM1)

// txM2 conflicts with txGood and signed by one malicious signer.
txM2 := newTestTx(t, m3.ScriptHash(), testScript)
txM2.Attributes = []transaction.Attribute{
{
Type: transaction.ConflictsT,
Value: &transaction.Conflicts{
Hash: txGood.Hash(),
},
},
}
txM2.NetworkFee = 1_000_0000
require.NoError(t, m3.SignTx(netmode.UnitTestNet, txM2))
e.AddNewBlock(t, txM2)

// We expect `tx` to pass verification, because on-chained `conflicting` doesn't have
// `tx`'s payer in the signers list, thus, `conflicting` should be considered as
// malicious conflict.
require.NoError(t, bc.VerifyTx(txGood))

// After that txGood can be added to the chain normally.
e.AddNewBlock(t, txGood)

// And after that ErrAlreadyExist is expected on verification.
require.ErrorIs(t, bc.VerifyTx(txGood), core.ErrAlreadyExists)
})

t.Run("multiple on-chain conflicts signed by [valid+malicious] parties", func(t *testing.T) {
m1 := e.NewAccount(t)
m2 := e.NewAccount(t)
m3 := e.NewAccount(t)
good := e.NewAccount(t)

// txGood doesn't conflict with anyone and signed by good signer.
txGood := newTestTx(t, good.ScriptHash(), testScript)
require.NoError(t, good.SignTx(netmode.UnitTestNet, txGood))

// txM1 conflicts with txGood and signed by one malicious and one good signers.
txM1 := newTestTx(t, m1.ScriptHash(), testScript)
txM1.Signers = append(txM1.Signers, transaction.Signer{Account: good.ScriptHash()})
txM1.Attributes = []transaction.Attribute{
{
Type: transaction.ConflictsT,
Value: &transaction.Conflicts{
Hash: txGood.Hash(),
},
},
}
txM1.NetworkFee = 1_000_0000
require.NoError(t, m1.SignTx(netmode.UnitTestNet, txM1))
require.NoError(t, good.SignTx(netmode.UnitTestNet, txM1))
e.AddNewBlock(t, txM1)

// txM2 conflicts with txGood and signed by two malicious signers.
txM2 := newTestTx(t, m2.ScriptHash(), testScript)
txM2.Signers = append(txM2.Signers, transaction.Signer{Account: m3.ScriptHash()})
txM2.Attributes = []transaction.Attribute{
{
Type: transaction.ConflictsT,
Value: &transaction.Conflicts{
Hash: txGood.Hash(),
},
},
}
txM2.NetworkFee = 1_000_0000
require.NoError(t, m2.SignTx(netmode.UnitTestNet, txM2))
require.NoError(t, m3.SignTx(netmode.UnitTestNet, txM2))
e.AddNewBlock(t, txM2)

// We expect `tx` to fail verification, because one of the on-chained `conflicting`
// transactions has common signers with `tx`, thus, `conflicting` should be
// considered as a valid conflict.
require.ErrorIs(t, bc.VerifyTx(txGood), core.ErrHasConflicts)
})

t.Run("multiple on-chain conflicts signed by [malicious+valid] parties", func(t *testing.T) {
m1 := e.NewAccount(t)
m2 := e.NewAccount(t)
m3 := e.NewAccount(t)
good := e.NewAccount(t)

// txGood doesn't conflict with anyone and signed by good signer.
txGood := newTestTx(t, good.ScriptHash(), testScript)
require.NoError(t, good.SignTx(netmode.UnitTestNet, txGood))

// txM2 conflicts with txGood and signed by two malicious signers.
txM2 := newTestTx(t, m2.ScriptHash(), testScript)
txM2.Signers = append(txM2.Signers, transaction.Signer{Account: m3.ScriptHash()})
txM2.Attributes = []transaction.Attribute{
{
Type: transaction.ConflictsT,
Value: &transaction.Conflicts{
Hash: txGood.Hash(),
},
},
}
txM2.NetworkFee = 1_000_0000
require.NoError(t, m2.SignTx(netmode.UnitTestNet, txM2))
require.NoError(t, m3.SignTx(netmode.UnitTestNet, txM2))
e.AddNewBlock(t, txM2)

// txM1 conflicts with txGood and signed by one malicious and one good signers.
txM1 := newTestTx(t, m1.ScriptHash(), testScript)
txM1.Signers = append(txM1.Signers, transaction.Signer{Account: good.ScriptHash()})
txM1.Attributes = []transaction.Attribute{
{
Type: transaction.ConflictsT,
Value: &transaction.Conflicts{
Hash: txGood.Hash(),
},
},
}
txM1.NetworkFee = 1_000_0000
require.NoError(t, m1.SignTx(netmode.UnitTestNet, txM1))
require.NoError(t, good.SignTx(netmode.UnitTestNet, txM1))
e.AddNewBlock(t, txM1)

// We expect `tx` to fail verification, because one of the on-chained `conflicting`
// transactions has common signers with `tx`, thus, `conflicting` should be
// considered as a valid conflict.
require.ErrorIs(t, bc.VerifyTx(txGood), core.ErrHasConflicts)
})

t.Run("multiple on-chain conflicts signed by [valid + malicious + valid] parties", func(t *testing.T) {
m1 := e.NewAccount(t)
m2 := e.NewAccount(t)
m3 := e.NewAccount(t)
good := e.NewAccount(t)

// txGood doesn't conflict with anyone and signed by good signer.
txGood := newTestTx(t, good.ScriptHash(), testScript)
require.NoError(t, good.SignTx(netmode.UnitTestNet, txGood))

// txM1 conflicts with txGood and signed by one malicious and one good signers.
txM1 := newTestTx(t, m1.ScriptHash(), testScript)
txM1.Signers = append(txM1.Signers, transaction.Signer{Account: good.ScriptHash()})
txM1.Attributes = []transaction.Attribute{
{
Type: transaction.ConflictsT,
Value: &transaction.Conflicts{
Hash: txGood.Hash(),
},
},
}
txM1.NetworkFee = 1_000_0000
require.NoError(t, m1.SignTx(netmode.UnitTestNet, txM1))
require.NoError(t, good.SignTx(netmode.UnitTestNet, txM1))
e.AddNewBlock(t, txM1)

// txM2 conflicts with txGood and signed by two malicious signers.
txM2 := newTestTx(t, m2.ScriptHash(), testScript)
txM2.Signers = append(txM2.Signers, transaction.Signer{Account: m3.ScriptHash()})
txM2.Attributes = []transaction.Attribute{
{
Type: transaction.ConflictsT,
Value: &transaction.Conflicts{
Hash: txGood.Hash(),
},
},
}
txM2.NetworkFee = 1_000_0000
require.NoError(t, m2.SignTx(netmode.UnitTestNet, txM2))
require.NoError(t, m3.SignTx(netmode.UnitTestNet, txM2))
e.AddNewBlock(t, txM2)

// txM3 conflicts with txGood and signed by one good and one malicious signers.
txM3 := newTestTx(t, good.ScriptHash(), testScript)
txM3.Signers = append(txM3.Signers, transaction.Signer{Account: m1.ScriptHash()})
txM3.Attributes = []transaction.Attribute{
{
Type: transaction.ConflictsT,
Value: &transaction.Conflicts{
Hash: txGood.Hash(),
},
},
}
txM3.NetworkFee = 1_000_0000
require.NoError(t, good.SignTx(netmode.UnitTestNet, txM3))
require.NoError(t, m1.SignTx(netmode.UnitTestNet, txM3))
e.AddNewBlock(t, txM3)

// We expect `tx` to fail verification, because one of the on-chained `conflicting`
// transactions has common signers with `tx`, thus, `conflicting` should be
// considered as a valid conflict.
require.ErrorIs(t, bc.VerifyTx(txGood), core.ErrHasConflicts)
})

t.Run("on-chain conflict signed by single valid sender", func(t *testing.T) {
tx := newTestTx(t, h, testScript)
tx.Signers = []transaction.Signer{{Account: validator.ScriptHash()}}
require.NoError(t, validator.SignTx(netmode.UnitTestNet, tx))
conflicting := transaction.New([]byte{byte(opcode.RET)}, 1000_0000)
conflicting.ValidUntilBlock = bc.BlockHeight() + 1
conflicting.Signers = []transaction.Signer{
{
Account: validator.ScriptHash(),
Scopes: transaction.CalledByEntry,
},
}
conflicting.Attributes = []transaction.Attribute{
{
Type: transaction.ConflictsT,
Value: &transaction.Conflicts{
Hash: tx.Hash(),
},
},
}
conflicting.NetworkFee = 1000_0000
require.NoError(t, validator.SignTx(netmode.UnitTestNet, conflicting))
e.AddNewBlock(t, conflicting)
// We expect `tx` to fail verification, because on-chained `conflicting` has
// `tx`'s payer as a signer.
require.ErrorIs(t, bc.VerifyTx(tx), core.ErrHasConflicts)
})
})
t.Run("attribute on-chain conflict", func(t *testing.T) {
tx := neoValidatorsInvoker.Invoke(t, stackitem.NewBool(true), "transfer", neoOwner, neoOwner, 1, nil)
Expand Down
Loading

0 comments on commit 081f9d3

Please sign in to comment.