It can't ever happen. We're guaranteed to have a consistent chain of headers
(we're verifying them above, if we're not verifying --- it's not our fault)
that starts at HeaderHeight that was actual when we were asking for it
previously. HeaderHeight can only move forward, so if that happened that would
be filtered out by the condition below and the first one can't happen. Though
to be absolutely sure change the second check to only pass "+1" headers (which
is what we want).
It's used in two places now:
* Blockchain.AddBlock()
This one does transaction duplication check of its own, doing it in
Verify() is just a waste of time. Merkle tree root hash value check is
still relevant though
* Block.DecodeBinary()
We're decoding blocks for the following purposes:
- on restore from dump
The block will be added to the chain via AddBlock() and that will do a
full check of it (if configured to do so)
- on retrieving the block from the DB (DAO)
We trust the DB, if it's gone wild, this check won't really help
- on receiving the block via P2P
It's gonna be put into block queue and then end up in AddBlock() which
will check it
- on receiving the block via RPC (submitblock)
It is to be passed into AddBlock()
- on receiving the block via RPC in a client
That's the only problematic case probably, but RPC client has to trust
the server and it can check for the signature if it really
cares. Or a separate in-client check might be added.
As we can see nothing really requires this verification to be done the way it
is now, AddBlock can just have a Merkle check and DecodeBinary can do fine
without it at all.
It's a no-op and there is nothing we can do about it, header contents could
only be checked against chain state, there is nothing to check for internal
consistency.
Now we have VerifyTx() and PoolTx() APIs that either verify transaction in
isolation or verify it against the mempool (either the primary one or the one
given) and then add it there. There is no possibility to check against the
mempool, but not add a transaction to it, but I doubt we really need it.
It allows to remove some duplication between old PoolTx and verifyTx where
they both tried to check transaction against mempool (verifying first and then
adding it). It also saves us utility token balance check because it's done by
the mempool anyway and we no longer need to do that explicitly in verifyTx.
It makes AddBlock() and verifyBlock() transaction's checks more correct,
because previously they could miss that even though sender S has enough
balance to pay for A, B or C, he can't pay for all of them.
Caveats:
* consensus is running concurrently to other processes, so things could
change while verifyBlock() is iterating over transactions, this will be
mitigated in subsequent commits
Improves TPS value for single node by at least 11%.
Fixes#667, fixes#668.
New transactions are added to the chain with blocks. If there is no
transaction X at height N in DAO, it could only be added with block N+1, so
it has to be present there. Therefore we can replace `dao.HasTransaction()`
check with a search through in-block transactions. HasTransaction() is nasty
in that it may add useless load the DB and this code is being run with a big
Blockchain lock held, so we don't want to be delayed here at all.
Improves single-node TPS by ~2%.
The end effect is almost as if `VerifyTransactions: false` was set in the
config, but without actually compromising the guarantees provided by it.
It almost doubles performance for single-mode benchmarks and makes block
processing smoother (more smaller blocks are being produced).
C# node is quite picky as it expects there to be exactly one value returned,
but our testchain actually adds 4 signatures for multisig cases instead of 3
which makes it technically incompatible with C# node.
We were checking blocked accounts twice which is obviously excessive. We also
have our accounts sorted, so we can rely on that in CheckPolicy(). It also
doesn't make much sense to check MaxBlockSystemFee in Blockchain code, policy
contract can handle that.
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).
It's not needed any more with Go 1.13 as we have wrapping/unwrapping in base
packages. All errors.Wrap calls are replaced with fmt.Errorf, some strings are
improved along the way.
We need to compact our in-memory MPT from time to time, otherwise it quickly
fills up all available memory. This raises two obvious quesions --- when to do
that and to what level do that.
As for 'when', I think it's quite easy to use our regular persistence interval
as an anchor (and it also frees up some memory), but we can't do that in the
persistence routine itself because of synchronization issues (adding some
synchronization primitives would add some cost that I'd also like to avoid),
so do it indirectly by comparing persisted and current height in `storeBlock`.
Choosing proper level is another problem, but if we're to roughly estimate one
full branch node to use 1K of memory (usually it's way less than that) then we
can easily store 1K of these nodes and that gives us a depth of 10 for our
trie.
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
This was differing from C# notion of PrevHash. It's not a previous root, but
rather a hash of the previous serialized MPTRoot structure (that is to be
signed by CNs).
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Disallow costly verification methods. We put this limit in policy
contract as it may be a subject to change in future.
In fact this value also overrides gas limit for header verification.
Close#1202.
We were accepting transactions with zero system fee, but we shouldn't do
that. Also, transaction's verification execution has to be limited by network
fee.
GetValidators without parameter is called upon DBFT initialization and it
should receive validators for the next block (that will create it),
parameterized GetValidators is used for NextConsensus calculation where we
need a list for the current state of the chain.
NextBlockValidators are updated before the new block persist, so we need to
use GetValidators to get the list corresponding to the current state of the
chain.
part of #904
1. We now have MaxTransactionsPerBlock set in native Policy contract,
so this value should be used in (dbft).GetVerified method instead
of passing it as an argument.
2. Removed (dbft).WithTxPerBlock.
2. DBFT API has changed, so update it's version.
3. Removed MaxTransactionsPerBlock from node configuration, as we
have it set in native Policy contract.
There is no such thing as high/low priority transactions, as there are
no free transactions anymore and they are ordered by fees contained
in transaction itself.
Closes#1063.
We make it explicit in the appropriate Block/Transaction structures, not via a
singleton as C# node does. I think this approach has a bit more potential and
allows better packages reuse for different purposes.
Two changes being done here, because they require a lot of updates to
tests. Now we're back into version 0 and we only have one type of
transaction.
It also removes GetType and GetScript interops, both are obsolete in Neo 3.
Getting batch, updating Prometheus metrics and pushing events doesn't require
any locking: batch is a local cache batch that no one outside cares about,
Prometheus metrics are not critical to be in perfect sync and events are
asynchronous anyway.
Native contracts also don't require any locks and they should be processed
before dumping storage changes.
Native contracts deployment creates `Transfer` notifications and adds
them into interop context. However, these notifications were not stored
for two reasons:
1. typo in `Transfer` (so these notifications were not recognised during
processing of the invocation tx in (*Blockchain).storeBlock(...) method)
2. these notifications have `from` adress setted to null, so conversion
to []byte fails. Same thing could happen with `to`.
Related C# issue: https://github.com/neo-project/neo/issues/1646
For now, made both `transfer` and `Transfer` valid.
The notion of NativeContractState shouldn't ever existed, native contract is a
contract and its state is saved as regular contract state which is critical
because we'll have MPT calculations over this state soon.
Initial minting should be done in Neo.Native.Deploy because it generates
notification that should have proper transaction context.
RegisterNative() shouldn't exist as a public method, native contracts are only
registered at block 0 and they can do it internally, no outside user should be
able to mess with it.
Move some structures from `native` package to `interop` also to avoid circular
references as interop.Context has to have a list of native contracts (exposing
them via Blockchainer is again too dangerous, it's too powerful tool).
1. closes#841
2. Commented out test cases where binary transaction are used.
These test cases marked with `TODO NEO3.0: Update binary` and need to be
updated.
3. Updated other tests.
4. Added cache to calculateValidUntilBlock() RPC-client method.
