From 791c983304d8e66132a747ca5d35c50a6c7eed96 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 6 Aug 2020 21:16:34 +0300 Subject: [PATCH] core: drop GetScriptHashesForVerifying It no longer depends on blockchain state and there can't ever be an error, in fact we can always iterate over signers, so copying these hashes doesn't make much sense at all as well as sorting arrays in verifyTxWitnesses (witnesses order must match signers order). --- pkg/core/blockchain.go | 41 ++++++--------------------- pkg/core/blockchainer/blockchainer.go | 1 - pkg/core/native/policy.go | 10 ++----- pkg/network/helper_test.go | 3 -- 4 files changed, 11 insertions(+), 44 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 62063c7ce..3b7e34dea 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1206,20 +1206,16 @@ func (bc *Blockchain) verifyTx(t *transaction.Transaction, block *block.Block) e if t.ValidUntilBlock <= height || t.ValidUntilBlock > height+transaction.MaxValidUntilBlockIncrement { return fmt.Errorf("transaction has expired. ValidUntilBlock = %d, current height = %d", t.ValidUntilBlock, height) } - hashes, err := bc.GetScriptHashesForVerifying(t) - if err != nil { - return err - } blockedAccounts, err := bc.contracts.Policy.GetBlockedAccountsInternal(bc.dao) if err != nil { return err } - for _, h := range hashes { + for _, signer := range t.Signers { i := sort.Search(len(blockedAccounts), func(i int) bool { - return !blockedAccounts[i].Less(h) + return !blockedAccounts[i].Less(signer.Account) }) - if i != len(blockedAccounts) && blockedAccounts[i].Equals(h) { - return fmt.Errorf("policy check failed: account %s is blocked", h.StringLE()) + if i != len(blockedAccounts) && blockedAccounts[i].Equals(signer.Account) { + return fmt.Errorf("policy check failed: account %s is blocked", signer.Account.StringLE()) } } maxBlockSystemFee := bc.contracts.Policy.GetMaxBlockSystemFeeInternal(bc.dao) @@ -1410,19 +1406,6 @@ func (bc *Blockchain) GetEnrollments() ([]state.Validator, error) { return bc.contracts.NEO.GetRegisteredValidators(bc.dao) } -// GetScriptHashesForVerifying returns all the ScriptHashes of a transaction which will be use -// to verify whether the transaction is bonafide or not. -// Golang implementation of GetScriptHashesForVerifying method in C# (https://github.com/neo-project/neo/blob/master/neo/Network/P2P/Payloads/Transaction.cs#L190) -func (bc *Blockchain) GetScriptHashesForVerifying(t *transaction.Transaction) ([]util.Uint160, error) { - hashesResult := make([]util.Uint160, len(t.Signers)) - for i, s := range t.Signers { - hashesResult[i] = s.Account - } - - return hashesResult, nil - -} - // GetTestVM returns a VM and a Store setup for a test run of some sort of code. func (bc *Blockchain) GetTestVM(tx *transaction.Transaction) *vm.VM { systemInterop := bc.newInteropContext(trigger.Application, bc.dao, nil, tx) @@ -1503,20 +1486,12 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa // not yet added into any block. // Golang implementation of VerifyWitnesses method in C# (https://github.com/neo-project/neo/blob/master/neo/SmartContract/Helper.cs#L87). func (bc *Blockchain) verifyTxWitnesses(t *transaction.Transaction, block *block.Block) error { - hashes, err := bc.GetScriptHashesForVerifying(t) - if err != nil { - return err + if len(t.Signers) != len(t.Scripts) { + return fmt.Errorf("number of signers doesn't match witnesses (%d vs %d)", len(t.Signers), len(t.Scripts)) } - - witnesses := t.Scripts - if len(hashes) != len(witnesses) { - return fmt.Errorf("expected len(hashes) == len(witnesses). got: %d != %d", len(hashes), len(witnesses)) - } - sort.Slice(hashes, func(i, j int) bool { return hashes[i].Less(hashes[j]) }) - sort.Slice(witnesses, func(i, j int) bool { return witnesses[i].ScriptHash().Less(witnesses[j].ScriptHash()) }) interopCtx := bc.newInteropContext(trigger.Verification, bc.dao, block, t) - for i := 0; i < len(hashes); i++ { - err := bc.verifyHashAgainstScript(hashes[i], &witnesses[i], interopCtx, false, t.NetworkFee) + for i := range t.Signers { + err := bc.verifyHashAgainstScript(t.Signers[i].Account, &t.Scripts[i], interopCtx, false, t.NetworkFee) if err != nil { return fmt.Errorf("witness #%d: %w", i, err) } diff --git a/pkg/core/blockchainer/blockchainer.go b/pkg/core/blockchainer/blockchainer.go index b1ff94f12..038e7f3d1 100644 --- a/pkg/core/blockchainer/blockchainer.go +++ b/pkg/core/blockchainer/blockchainer.go @@ -43,7 +43,6 @@ type Blockchainer interface { GetNEP5Balances(util.Uint160) *state.NEP5Balances GetValidators() ([]*keys.PublicKey, error) GetStandByValidators() keys.PublicKeys - GetScriptHashesForVerifying(*transaction.Transaction) ([]util.Uint160, error) GetStateRoot(height uint32) (*state.MPTRootState, error) GetStorageItem(id int32, key []byte) *state.StorageItem GetStorageItems(id int32) (map[string]*state.StorageItem, error) diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 46abf3b40..254a776b7 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -516,14 +516,10 @@ func (p *Policy) CheckPolicy(ic *interop.Context, tx *transaction.Transaction) e if err != nil { return fmt.Errorf("unable to get blocked accounts list: %w", err) } - scriptHashes, err := ic.Chain.GetScriptHashesForVerifying(tx) - if err != nil { - return fmt.Errorf("unable to get tx script hashes: %w", err) - } for _, acc := range ba { - for _, hash := range scriptHashes { - if acc.Equals(hash) { - return fmt.Errorf("account %s is blocked", hash.StringLE()) + for _, signer := range tx.Signers { + if acc.Equals(signer.Account) { + return fmt.Errorf("account %s is blocked", acc.StringLE()) } } } diff --git a/pkg/network/helper_test.go b/pkg/network/helper_test.go index 45ccc3615..d4f2669a3 100644 --- a/pkg/network/helper_test.go +++ b/pkg/network/helper_test.go @@ -109,9 +109,6 @@ func (chain testChain) GetStandByValidators() keys.PublicKeys { func (chain testChain) GetEnrollments() ([]state.Validator, error) { panic("TODO") } -func (chain testChain) GetScriptHashesForVerifying(*transaction.Transaction) ([]util.Uint160, error) { - panic("TODO") -} func (chain testChain) GetStateRoot(height uint32) (*state.MPTRootState, error) { panic("TODO") }