Merge pull request #1439 from nspcc-dev/core/verify_tx_witnesses_fix

core: take into account gasConsumed during tx witnesses verification
This commit is contained in:
Roman Khimov 2020-09-29 18:11:39 +03:00 committed by GitHub
commit adcbb2287f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 34 additions and 21 deletions

View file

@ -1504,11 +1504,12 @@ func initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transac
func (bc *Blockchain) VerifyWitness(h util.Uint160, c crypto.Verifiable, w *transaction.Witness, gas int64) error { func (bc *Blockchain) VerifyWitness(h util.Uint160, c crypto.Verifiable, w *transaction.Witness, gas int64) error {
ic := bc.newInteropContext(trigger.Verification, bc.dao, nil, nil) ic := bc.newInteropContext(trigger.Verification, bc.dao, nil, nil)
ic.Container = c ic.Container = c
return bc.verifyHashAgainstScript(h, w, ic, gas) _, err := bc.verifyHashAgainstScript(h, w, ic, gas)
return err
} }
// verifyHashAgainstScript verifies given hash against the given witness. // verifyHashAgainstScript verifies given hash against the given witness and returns the amount of GAS consumed.
func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transaction.Witness, interopCtx *interop.Context, gas int64) error { func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transaction.Witness, interopCtx *interop.Context, gas int64) (int64, error) {
gasPolicy := bc.contracts.Policy.GetMaxVerificationGas(interopCtx.DAO) gasPolicy := bc.contracts.Policy.GetMaxVerificationGas(interopCtx.DAO)
if gas > gasPolicy { if gas > gasPolicy {
gas = gasPolicy gas = gasPolicy
@ -1518,28 +1519,28 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa
vm.SetPriceGetter(getPrice) vm.SetPriceGetter(getPrice)
vm.GasLimit = gas vm.GasLimit = gas
if err := initVerificationVM(interopCtx, hash, witness); err != nil { if err := initVerificationVM(interopCtx, hash, witness); err != nil {
return err return 0, err
} }
err := vm.Run() err := vm.Run()
if vm.HasFailed() { if vm.HasFailed() {
return fmt.Errorf("%w: vm execution has failed: %v", ErrVerificationFailed, err) return 0, fmt.Errorf("%w: vm execution has failed: %v", ErrVerificationFailed, err)
} }
resEl := vm.Estack().Pop() resEl := vm.Estack().Pop()
if resEl != nil { if resEl != nil {
res, err := resEl.Item().TryBool() res, err := resEl.Item().TryBool()
if err != nil { if err != nil {
return fmt.Errorf("%w: invalid return value", ErrVerificationFailed) return 0, fmt.Errorf("%w: invalid return value", ErrVerificationFailed)
} }
if !res { if !res {
return fmt.Errorf("%w: invalid signature", ErrVerificationFailed) return 0, fmt.Errorf("%w: invalid signature", ErrVerificationFailed)
} }
if vm.Estack().Len() != 0 { if vm.Estack().Len() != 0 {
return fmt.Errorf("%w: expected exactly one returned value", ErrVerificationFailed) return 0, fmt.Errorf("%w: expected exactly one returned value", ErrVerificationFailed)
} }
} else { } else {
return fmt.Errorf("%w: no result returned from the script", ErrVerificationFailed) return 0, fmt.Errorf("%w: no result returned from the script", ErrVerificationFailed)
} }
return nil return vm.GasConsumed(), nil
} }
// verifyTxWitnesses verifies the scripts (witnesses) that come with a given // verifyTxWitnesses verifies the scripts (witnesses) that come with a given
@ -1553,11 +1554,13 @@ func (bc *Blockchain) verifyTxWitnesses(t *transaction.Transaction, block *block
return fmt.Errorf("%w: %d vs %d", ErrTxInvalidWitnessNum, len(t.Signers), len(t.Scripts)) return fmt.Errorf("%w: %d vs %d", ErrTxInvalidWitnessNum, len(t.Signers), len(t.Scripts))
} }
interopCtx := bc.newInteropContext(trigger.Verification, bc.dao, block, t) interopCtx := bc.newInteropContext(trigger.Verification, bc.dao, block, t)
gasLimit := t.NetworkFee
for i := range t.Signers { for i := range t.Signers {
err := bc.verifyHashAgainstScript(t.Signers[i].Account, &t.Scripts[i], interopCtx, t.NetworkFee) gasConsumed, err := bc.verifyHashAgainstScript(t.Signers[i].Account, &t.Scripts[i], interopCtx, gasLimit)
if err != nil { if err != nil {
return fmt.Errorf("witness #%d: %w", i, err) return fmt.Errorf("witness #%d: %w", i, err)
} }
gasLimit -= gasConsumed
} }
return nil return nil

View file

@ -215,7 +215,7 @@ func TestVerifyTx(t *testing.T) {
bc := newTestChain(t) bc := newTestChain(t)
defer bc.Close() defer bc.Close()
accs := make([]*wallet.Account, 3) accs := make([]*wallet.Account, 4)
for i := range accs { for i := range accs {
var err error var err error
accs[i], err = wallet.NewAccount() accs[i], err = wallet.NewAccount()
@ -326,6 +326,16 @@ func TestVerifyTx(t *testing.T) {
tx.Scripts[0].InvocationScript[10] = ^tx.Scripts[0].InvocationScript[10] tx.Scripts[0].InvocationScript[10] = ^tx.Scripts[0].InvocationScript[10]
checkErr(t, ErrVerificationFailed, tx) checkErr(t, ErrVerificationFailed, tx)
}) })
t.Run("InsufficientNetworkFeeForSecondWitness", func(t *testing.T) {
tx := bc.newTestTx(h, testScript)
tx.Signers = append(tx.Signers, transaction.Signer{
Account: accs[3].PrivateKey().GetScriptHash(),
Scopes: transaction.Global,
})
require.NoError(t, accs[0].SignTx(tx))
require.NoError(t, accs[3].SignTx(tx))
checkErr(t, ErrVerificationFailed, tx)
})
t.Run("OldTX", func(t *testing.T) { t.Run("OldTX", func(t *testing.T) {
tx := bc.newTestTx(h, testScript) tx := bc.newTestTx(h, testScript)
require.NoError(t, accs[0].SignTx(tx)) require.NoError(t, accs[0].SignTx(tx))
@ -492,22 +502,22 @@ func TestVerifyHashAgainstScript(t *testing.T) {
newH := cs.ScriptHash() newH := cs.ScriptHash()
newH[0] = ^newH[0] newH[0] = ^newH[0]
w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH4)}} w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH4)}}
err := bc.verifyHashAgainstScript(newH, w, ic, gas) _, err := bc.verifyHashAgainstScript(newH, w, ic, gas)
require.True(t, errors.Is(err, ErrUnknownVerificationContract)) require.True(t, errors.Is(err, ErrUnknownVerificationContract))
}) })
t.Run("Invalid", func(t *testing.T) { t.Run("Invalid", func(t *testing.T) {
w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH4)}} w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH4)}}
err := bc.verifyHashAgainstScript(csInvalid.ScriptHash(), w, ic, gas) _, err := bc.verifyHashAgainstScript(csInvalid.ScriptHash(), w, ic, gas)
require.True(t, errors.Is(err, ErrInvalidVerificationContract)) require.True(t, errors.Is(err, ErrInvalidVerificationContract))
}) })
t.Run("ValidSignature", func(t *testing.T) { t.Run("ValidSignature", func(t *testing.T) {
w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH4)}} w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH4)}}
err := bc.verifyHashAgainstScript(cs.ScriptHash(), w, ic, gas) _, err := bc.verifyHashAgainstScript(cs.ScriptHash(), w, ic, gas)
require.NoError(t, err) require.NoError(t, err)
}) })
t.Run("InvalidSignature", func(t *testing.T) { t.Run("InvalidSignature", func(t *testing.T) {
w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH3)}} w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH3)}}
err := bc.verifyHashAgainstScript(cs.ScriptHash(), w, ic, gas) _, err := bc.verifyHashAgainstScript(cs.ScriptHash(), w, ic, gas)
require.True(t, errors.Is(err, ErrVerificationFailed)) require.True(t, errors.Is(err, ErrVerificationFailed))
}) })
}) })
@ -517,7 +527,7 @@ func TestVerifyHashAgainstScript(t *testing.T) {
InvocationScript: []byte{byte(opcode.NOP)}, InvocationScript: []byte{byte(opcode.NOP)},
VerificationScript: verif, VerificationScript: verif,
} }
err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, 1) _, err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, 1)
require.True(t, errors.Is(err, ErrVerificationFailed)) require.True(t, errors.Is(err, ErrVerificationFailed))
}) })
t.Run("NoResult", func(t *testing.T) { t.Run("NoResult", func(t *testing.T) {
@ -526,7 +536,7 @@ func TestVerifyHashAgainstScript(t *testing.T) {
InvocationScript: []byte{byte(opcode.PUSH1)}, InvocationScript: []byte{byte(opcode.PUSH1)},
VerificationScript: verif, VerificationScript: verif,
} }
err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, gas) _, err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, gas)
require.True(t, errors.Is(err, ErrVerificationFailed)) require.True(t, errors.Is(err, ErrVerificationFailed))
}) })
t.Run("BadResult", func(t *testing.T) { t.Run("BadResult", func(t *testing.T) {
@ -537,7 +547,7 @@ func TestVerifyHashAgainstScript(t *testing.T) {
InvocationScript: []byte{byte(opcode.NOP)}, InvocationScript: []byte{byte(opcode.NOP)},
VerificationScript: verif, VerificationScript: verif,
} }
err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, gas) _, err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, gas)
require.True(t, errors.Is(err, ErrVerificationFailed)) require.True(t, errors.Is(err, ErrVerificationFailed))
}) })
t.Run("TooManyResults", func(t *testing.T) { t.Run("TooManyResults", func(t *testing.T) {
@ -546,7 +556,7 @@ func TestVerifyHashAgainstScript(t *testing.T) {
InvocationScript: []byte{byte(opcode.PUSH1), byte(opcode.PUSH1)}, InvocationScript: []byte{byte(opcode.PUSH1), byte(opcode.PUSH1)},
VerificationScript: verif, VerificationScript: verif,
} }
err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, gas) _, err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, gas)
require.True(t, errors.Is(err, ErrVerificationFailed)) require.True(t, errors.Is(err, ErrVerificationFailed))
}) })
} }

View file

@ -40,7 +40,7 @@ func checkScope(d dao.DAO, tx *transaction.Transaction, v *vm.VM, hash util.Uint
if c.Scopes&transaction.CalledByEntry != 0 { if c.Scopes&transaction.CalledByEntry != 0 {
callingScriptHash := v.GetCallingScriptHash() callingScriptHash := v.GetCallingScriptHash()
entryScriptHash := v.GetEntryScriptHash() entryScriptHash := v.GetEntryScriptHash()
if callingScriptHash == entryScriptHash { if callingScriptHash.Equals(util.Uint160{}) || callingScriptHash == entryScriptHash {
return true, nil return true, nil
} }
} }