The CSV consensus rules dictate that the opcode fails when the
transaction version is not at least version 2, however that only applies
if the disable flag is not set in the sequence.
This is not an issue at the current time because we do not yet enforce
CSV at a consensus level, however, I noticed this discrepancy when doing
a thorough audit of the CSV paths due to the ongoing work to add full
consensus-enforced CSV support.
As a result, this must be merged prior to enabling consensus enforcement
for CSV or it would open up the potential for a hard fork.
This is mostly a backport of some of the same modifications made in
Decred along with a few additional things cleaned up. In particular,
this updates the code to make use of the new chainhash package.
Also, since this required API changes anyways and the hash algorithm is
no longer tied specifically to SHA, all other functions throughout the
code base which had "Sha" in their name have been changed to Hash so
they are not incorrectly implying the hash algorithm.
The following is an overview of the changes:
- Remove the wire.ShaHash type
- Update all references to wire.ShaHash to the new chainhash.Hash type
- Rename the following functions and update all references:
- wire.BlockHeader.BlockSha -> BlockHash
- wire.MsgBlock.BlockSha -> BlockHash
- wire.MsgBlock.TxShas -> TxHashes
- wire.MsgTx.TxSha -> TxHash
- blockchain.ShaHashToBig -> HashToBig
- peer.ShaFunc -> peer.HashFunc
- Rename all variables that included sha in their name to include hash
instead
- Update for function name changes in other dependent packages such as
btcutil
- Update copyright dates on all modified files
- Update glide.lock file to use the required version of btcutil
See https://github.com/bitcoin/bips/blob/master/bip-0065.mediawiki for
more information.
This commit mimics Bitcoin Core commit bc60b2b4b401f0adff5b8b9678903ff8feb5867b
and includes additional tests from Bitcoin Core commit
cb54d17355864fa08826d6511a0d7692b21ef2c9
Introduce an ECDSA signature verification into btcd in order to
mitigate a certain DoS attack and as a performance optimization.
The benefits of SigCache are two fold. Firstly, usage of SigCache
mitigates a DoS attack wherein an attacker causes a victim's client to
hang due to worst-case behavior triggered while processing attacker
crafted invalid transactions. A detailed description of the mitigated
DoS attack can be found here: https://bitslog.wordpress.com/2013/01/23/fixed-bitcoin-vulnerability-explanation-why-the-signature-cache-is-a-dos-protection/
Secondly, usage of the SigCache introduces a signature verification
optimization which speeds up the validation of transactions within a
block, if they've already been seen and verified within the mempool.
The server itself manages the sigCache instance. The blockManager and
txMempool respectively now receive pointers to the created sigCache
instance. All read (sig triplet existence) operations on the sigCache
will not block unless a separate goroutine is adding an entry (writing)
to the sigCache. GetBlockTemplate generation now also utilizes the
sigCache in order to avoid unnecessarily double checking signatures
when generating a template after previously accepting a txn to the
mempool. Consequently, the CPU miner now also employs the same
optimization.
The maximum number of entries for the sigCache has been introduced as a
config parameter in order to allow users to configure the amount of
memory consumed by this new additional caching.
This commit contains fixes from the results of a thorough audit of
txscript to find any cases of script evaluation which doesn't match the
required consensus behavior. These conditions are fairly obscure and
highly unlikely to happen in any real scripts, but they could have
nevertheless been used by a clever attacker with malicious intent to
cause a fork.
Test cases which exercise these conditions have been added to the
reference tests and will contributed upstream to improve the quality for
the entire ecosystem.
Unlike OP_IF and OP_NOTIF which interpret the top stack item as a
number, OP_IFDUP interprets it as a boolean. This has important
consequences because numbers are imited to int32s while booleans can be
an arbitrary number of bytes.
The offending script was found and reported by Jonas Nick through the
use of fuzzing.
This commit implements a new type, named scriptNum, for handling all
numeric values used in scripts and converts the code over to make use of
it. This is being done for a few of reasons.
First, the consensus rules for handling numeric values in the scripts
require special handling with subtle semantics. By encapsulating those
details into a type specifically dedicated to that purpose, it
simplifies the code and generally helps prevent improper usage.
Second, the new type is quite a bit more efficient than big.Ints which
are designed to be arbitrarily large and thus involve a lot of heap
allocations and additional multi-precision bookkeeping. Because this
new type is based on an int64, it allows the numbers to be stack
allocated thereby eliminating a lot of GC and also eliminates the extra
multi-precision arithmetic bookkeeping.
The use of an int64 is possible because the consensus rules dictate that
when data is interpreted as a number, it is limited to an int32 even
though results outside of this range are allowed so long as they are not
interpreted as integers again themselves. Thus, the maximum possible
result comes from multiplying a max int32 by itself which safely fits
into an int64 and can then still appropriately provide the serialization
of the larger number as required by consensus.
Finally, it more closely resembles the implementation used by Bitcoin
Core and thus makes is easier to compare the behavior between the two
implementations.
This commit also includes a full suite of tests with 100% coverage of
the semantics of the new type.
This commit contains a lot of cleanup on the txscript code to make it
more consistent with the code throughout the rest of the project. It
doesn't change any operational logic.
The following is an overview of the changes:
- Add a significant number of comments throughout in order to better
explain what the code is doing
- Fix several comment typos
- Move a couple of constants only used by the engine to engine.go
- Move a variable only used by the engine to engine.go
- Fix a couple of format specifiers in the test prints
- Reorder functions so they're defined before/closer to use
- Make the code lint clean with the exception of the opcode definitions
- Remove all redundant opcode tests in favor of the JSON-based tests
in the data directory.
- Remove duplicate stack nip test
- Add new tests to data/script_invalid.json to exercise additional
negative error paths
- Remove old unneeded pubkey trace code from opcodeCheckSig
- Simplify and improve the disassembly print function
- Add new tests to directly test all individual opcode disassembly
- Add new tests to directly test opcode disabled function which does not
get invoked during ordinary execution
- Improve test coverage of opcode.go
This commit moves the opcode execution logic from the opcode type to the
engine type because execution of an opcode modifies the engine state
(primarily the main and alternate data stacks) as opposed to the state
of the opcode. Making the engine the receiver more clearly indicates
this fact.
This commit very slightly optimizes the cryptographic hashing performed
by the script opcodes by calling the hash sum routines directly (for
those that support it) rather than allocating a new generic hash.Hash
hasher instance for them.
This commit improves the way the conditional execution stack is handled in
a few ways.
First, the current execution state is now pushed onto the end of the slice
rather than the front of it. This has been done because it results in
fewer allocations and is therefore more efficient.
Second, the need for allocating and setting an initial true in the
conditional stack has been eliminated. The vast majority of scripts don't
contain any conditionals, so there is no reason to allocate a slice when
it isn't needed.
Third, a new function has been added to the engine to determine if the
current conditional branch is executing named isBranchExecuting which
handles the fact the conditional execution stack can now be empty and
improves the readability of the code.
Finally, it removes a couple of TODOs which I have verified do not apply.
This commit exports a new map named OpcodeByName which can be used to
lookup an opcode value given a human-readable opcode name.
It also modifies the test function which does short form parsing to use
the new map instead of the internal array.
Closes#267.
This commit converts the opcode map to an array to improve performance.
Benchmark of executing a standard p2pk transaction:
New: BenchmarkExecute 2000 784349 ns/op
Old: BenchmarkExecute 2000 792600 ns/op
The time is dominated by the signature checking as expected, however there
is still an increase in speed.
This commit modifies the definition of the opcodes to their hex
counterparts rather than decimal since it is far more common to see
scripts in hex. This makes it easier when manually looking at script
dumps to correlate opcodes. However, since there are also cases where it
is useful to see the decimal value of the opcode, the decimal value has
been left as a comment. Obviously converting the numbers is trivial, but
it is handy when looking at the opcode definitions to already have it
there.
In addition, it syncs the opcodes with the latest Bitcoin Core internal
opcodes for completeness and modifies the tests accordingly.
Rather than storing a separate bool for whether or not each flag is set in
every script engine instance, store the flags and check if the relevant
flag is set from each specific location.
This reduces the memory needed by each script engine instance and means
future flags will not require new fields.
This commit renames the Script type to Engine to better reflect its
purpose. It also renames the NewScript function to NewEngine to match.
This is being done because name Script for the engine is confusing since
it implies it is an actual script rather than the execution environment
for the script. It also paves the way for eventually supplying a
ParsedScript type which will be less likely to be confused with the
execution environment.
While moving the code, some additional variable names and comments have
been updated to better match the style used throughout the rest of the
code base. In addition, an attempt has been made to use consistent naming
of the engine as 'vm' instead of using different variables names as it was
previously.
Finally, the relevant engine code has been moved into a new file named
engine.go and related tests moved to engine_test.go.
The ScriptVerifyMinimalData enforces that all push operations use the
minimal data push required. This is part of BIP0062.
This commit mimics Bitcoin Core commit
698c6abb25c1fbbc7fa4ba46b60e9f17d97332ef
Remove ScriptCanonicalSignatures and use the new
ScriptVerifyDERSignatures flag. The ScriptVerifyDERSignatures
flag accomplishes the same functionality.
This commit adds two new verification flags to txscript named
ScriptVerifyStrictEncoding and ScriptVerifyDerSignatures.
The ScriptVerifyStrictEncoding flag enforces signature scripts
and public keys to follow the strict encoding requirements.
The ScriptVerifyDerSignatures flag enforces signature scripts
to follow the strict encoding requirements.
These flags mimic Bitcoin Core's SCRIPT_VERIFY_STRICTENC and
SCRIPT_VERIFY_DERSIG flags and brings the Bitcoin Core test scripts up
to date.
This commit corrects a case in the OP_CHECKMULTISIG handling where it was
possible to improperly validate a transaction that had a combination of
valid and malformed signatures.
It also adds a new test to ensure this case is properly handled and nukes
a superfluous comment.
Fixes#293.
This commit contains the entire btcscript repository along with several
changes needed to move all of the files into the txscript directory in
order to prepare it for merging. This does NOT update btcd or any of the
other packages to use the new location as that will be done separately.
- All import paths in the old btcscript test files have been changed to the
new location
- All references to btcscript as the package name have been chagned to
txscript
This is ongoing work toward #214.
