Skip to content

Commit

Permalink
*: add DefaultInvocationScriptSize
Browse files Browse the repository at this point in the history
Magical numbers are not allowed.

Signed-off-by: Ekaterina Pavlova <[email protected]>
  • Loading branch information
AliceInHunterland committed Jan 29, 2025
1 parent 646625b commit 4a9c2eb
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 9 deletions.
2 changes: 1 addition & 1 deletion pkg/core/blockchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -2510,7 +2510,7 @@ func (bc *Blockchain) ApplyPolicyToTxSet(txes []*transaction.Transaction) []*tra
m := smartcontract.GetDefaultHonestNodeCount(curVC)
verification, _ := smartcontract.CreateDefaultMultiSigRedeemScript(bc.contracts.NEO.GetNextBlockValidatorsInternal(bc.dao))
defaultWitness = transaction.Witness{
InvocationScript: make([]byte, 66*m),
InvocationScript: make([]byte, transaction.DefaultInvocationScriptSize*m),
VerificationScript: verification,
}
bc.knownValidatorsCount.Store(curVC)
Expand Down
2 changes: 1 addition & 1 deletion pkg/core/blockchain_neotest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2241,7 +2241,7 @@ func TestBlockchain_VerifyTx(t *testing.T) {
tx.NetworkFee = netFee + // multisig witness verification price
int64(size)*bc.FeePerByte() + // fee for unsigned size
int64(sizeDelta)*bc.FeePerByte() + // fee for multisig size
66*bc.FeePerByte() + // fee for Notary signature size (66 bytes for Invocation script and 0 bytes for Verification script)
transaction.DefaultInvocationScriptSize*bc.FeePerByte() + // fee for Notary signature size (66 bytes for Invocation script and 0 bytes for Verification script)
2*bc.FeePerByte() + // fee for the length of each script in Notary witness (they are nil, so we did not take them into account during `size` calculation)
notaryServiceFeePerKey + // fee for Notary attribute
fee.Opcode(bc.GetBaseExecFee(), // Notary verification script
Expand Down
3 changes: 2 additions & 1 deletion pkg/core/fee/calculate.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package fee

import (
"github.com/nspcc-dev/neo-go/pkg/core/transaction"
"github.com/nspcc-dev/neo-go/pkg/io"
"github.com/nspcc-dev/neo-go/pkg/vm"
"github.com/nspcc-dev/neo-go/pkg/vm/emit"
Expand All @@ -21,7 +22,7 @@ func Calculate(base int64, script []byte) (int64, int) {
netFee += Opcode(base, opcode.PUSHDATA1, opcode.PUSHDATA1) + base*ECDSAVerifyPrice
} else if m, pubs, ok := vm.ParseMultiSigContract(script); ok {
n := len(pubs)
sizeInv := 66 * m
sizeInv := transaction.DefaultInvocationScriptSize * m
size += io.GetVarSize(sizeInv) + sizeInv + io.GetVarSize(script)
netFee += calculateMultisig(base, m) + calculateMultisig(base, n)
netFee += base * ECDSAVerifyPrice * int64(n)
Expand Down
4 changes: 4 additions & 0 deletions pkg/core/transaction/witness.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ const (
// MaxVerificationScript is the maximum allowed length of verification
// script. It should be appropriate for 11/21 multisignature committee.
MaxVerificationScript = 1024

// DefaultInvocationScriptSize is the default length of invocation
// script with one signature.
DefaultInvocationScriptSize = 66
)

// Witness contains 2 scripts.
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/payload/notary_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (r *P2PNotaryRequest) isValid() error {
if len(r.FallbackTransaction.Signers) != 2 {
return errors.New("fallback transaction should have two signers")
}
if len(r.FallbackTransaction.Scripts[0].InvocationScript) != 66 ||
if len(r.FallbackTransaction.Scripts[0].InvocationScript) != transaction.DefaultInvocationScriptSize ||
len(r.FallbackTransaction.Scripts[0].VerificationScript) != 0 ||
!bytes.HasPrefix(r.FallbackTransaction.Scripts[0].InvocationScript, []byte{byte(opcode.PUSHDATA1), keys.SignatureLen}) {
return errors.New("fallback transaction has invalid dummy Notary witness")
Expand Down
2 changes: 1 addition & 1 deletion pkg/services/notary/notary.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ func (n *Notary) verifyIncompleteWitnesses(tx *transaction.Transaction, nKeysExp
}
// Each verification script is allowed to have either one signature or zero signatures. If signature is provided, then need to verify it.
if len(w.InvocationScript) != 0 {
if len(w.InvocationScript) != 66 || !bytes.HasPrefix(w.InvocationScript, []byte{byte(opcode.PUSHDATA1), keys.SignatureLen}) {
if len(w.InvocationScript) != transaction.DefaultInvocationScriptSize || !bytes.HasPrefix(w.InvocationScript, []byte{byte(opcode.PUSHDATA1), keys.SignatureLen}) {
return nil, fmt.Errorf("witness #%d: invocation script should have length = 66 and be of the form [PUSHDATA1, 64, signatureBytes...]", i)
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/wallet/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,11 @@ func TestContractSignTx(t *testing.T) {

require.NoError(t, acc2.SignTx(0, tx)) // Append script for acc2.
require.Equal(t, 1, len(tx.Scripts))
require.Equal(t, 66, len(tx.Scripts[0].InvocationScript))
require.Equal(t, transaction.DefaultInvocationScriptSize, len(tx.Scripts[0].InvocationScript))

require.NoError(t, acc2.SignTx(0, tx)) // Sign again, effectively a no-op.
require.Equal(t, 1, len(tx.Scripts))
require.Equal(t, 66, len(tx.Scripts[0].InvocationScript))
require.Equal(t, transaction.DefaultInvocationScriptSize, len(tx.Scripts[0].InvocationScript))

acc2.Locked = true
require.False(t, acc2.CanSign())
Expand All @@ -154,12 +154,12 @@ func TestContractSignTx(t *testing.T) {
VerificationScript: acc.Contract.Script,
})
require.NoError(t, acc.SignTx(0, tx)) // Add invocation script for existing witness.
require.Equal(t, 66, len(tx.Scripts[1].InvocationScript))
require.Equal(t, transaction.DefaultInvocationScriptSize, len(tx.Scripts[1].InvocationScript))
require.NotNil(t, acc.SignHashable(0, tx)) // Works via Hashable too.

require.NoError(t, multiAcc.SignTx(0, tx))
require.Equal(t, 3, len(tx.Scripts))
require.Equal(t, 66, len(tx.Scripts[2].InvocationScript))
require.Equal(t, transaction.DefaultInvocationScriptSize, len(tx.Scripts[2].InvocationScript))

require.NoError(t, multiAcc2.SignTx(0, tx)) // Append to existing script.
require.Equal(t, 3, len(tx.Scripts))
Expand Down

0 comments on commit 4a9c2eb

Please sign in to comment.