This was done in https://github.com/neo-project/neo/pull/1693
for native calls. `OnPersist` script still uses `DROP` though
as value is pushed via `CheckReturn` logic for regular calls.
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.
NewMerkleTree is a memory hog, we can do better than that:
BenchmarkMerkle/NewMerkleTree-8 13 88434670 ns/op 20828207 B/op 300035 allocs/op
BenchmarkMerkle/CalcMerkleRoot-8 15 69264150 ns/op 0 B/op 0 allocs/op
It's not needed, we're either creating a new one and assigning it 6 lines
above or we're changing already existing big.Int via a pointer, so no update
is needed.
There is nothing requiring us to do so. It also is bad because it allows for
new transaction to replace some already existing one with the same fee
parameters just because it has "better" hash.
But the other thing is that for transactions with equal fees it's always
better for us to append them to the end of the list, instead of inserting them
in the middle, so this change allows to reduce slice item movements and gain
some 6-7% increase for single-node TPS.
Time is not really relevant for us here and we don't use this timestamp in any
way. Yet it occupies 24 bytes and we do two clock_gettime calls to get it.
Replace it with blockStamp which is going to be used in the future for
transaction retransmissions.
It allows to improve single-node TPS by another 3%.
We're constantly checking for transactions there and most of the time this
check is not successful (meaning that the transaction in question is
new). Bloom filter easily reduces the need to search over the DB in 99% of
these cases and gives some 13% increase in single-node TPS.
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).
According to manifest, OnPersist.ReturnType is void, so we shouldn't
return anything from it. It's not so important, as we drop this value at
the end of OnPersist invocation.
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 currently can't process events in codegen, so we have to provide
them via .yml config file. Do not delete the rest of the code connected
with conversion of MethodDebugInfo.Event into manifest.Event as we have
issue #1038.
As it's returned sorted now. Fixes state change mismatch for
NextValidators. It also partially reverts
2f8e7e4d33 and significantly changes the test
chain as the fees are no longer being sent to the same account.
When calling external contracts we expect exactly 1 value to be on
stack. For methods returning nothing, `Null` value is pushed, otherwise
it is an error.`
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.
In 121c9664b we should take into account isValid flag of
NativePolicy contract while retrieving MaxVerificationGas native
policy value. Otherwise we won't be able to get MaxVerificationGas
after the node was restarted, because this value is not truly
stored along with the other native policy values.
This commit fixes bug with headers verification after the node
restarting with an existing storage:
```
2020-08-03T12:52:56.158+0300 WARN failed processing headers {"error": "vm failed to execute the script with error: error encountered at instruction 0 (PUSHDATA1): gas limit is exceeded", "errorVerbose": "vm failed to execute the script with error: error encountered at instruction 0 (PUSHDATA1): gas limit is exceeded\ngithub.com/nspcc-dev/neo-go/pkg/core.(*Blockchain).verifyHashAgainstScript\n\t/home/neospcc/Documents/GitProjects/nspcc-dev/neo-go/pkg/core/blockchain.go:1454\ngithub.com/nspcc-dev/neo-go/pkg/core.(*Blockchain).verifyHeaderWitnesses\n\t/home/neospcc/Documents/GitProjects/nspcc-dev/neo-go/pkg/core/blockchain.go:1517\ngithub.com/nspcc-dev/neo-go/pkg/core.(*Blockchain).verifyHeader\n\t/home/neospcc/Documents/GitProjects/nspcc-dev/neo-go/pkg/core/blockchain.go:1175\ngithub.com/nspcc-dev/neo-go/pkg/core.(*Blockchain).addHeaders\n\t/home/neospcc/Documents/GitProjects/nspcc-dev/neo-go/pkg/core/blockchain.go:484\ngithub.com/nspcc-dev/neo-go/pkg/core.(*Blockchain).AddHeaders\n\t/home/neospcc/Documents/GitProjects/nspcc-dev/neo-go/pkg/core/blockchain.go:453\ngithub.com/nspcc-dev/neo-go/pkg/network.(*Server).handleHeadersCmd\n\t/home/neospcc/Documents/GitProjects/nspcc-dev/neo-go/pkg/network/server.go:454\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1373"}
```
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>
Because trie size is rather big, it can't be stored in memory.
Thus some form of caching should also be implemented. To avoid
marshaling/unmarshaling of items which are close to root and are used
very frequenly we can save them across the persists.
This commit implements pruning items at the specified depth,
replacing them by hash nodes.
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Because there is no distinct type field in JSONized nodes, distinction
is made via payload itself, thus all unmarshaling is done via
NodeObject.
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
MPT is a trie with a branching factor = 16, i.e. it consists of sequences in
16-element alphabet.
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Get 2 contracts in pair which is useful everytime we need to test
syscall with one contract calling the other.
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Invoke `_initialize` method on every call if present.
In NEO3 there is no entrypoint and methods are invoked by offset,
thus `Main` function is no longer required.
We still have special `Main` method in tests to simplify them.
Allow to invoke methods by offset:
1. Every invoked contract must have manifest.
2. Check arguments count on invocation.
3. Change AppCall to a regular syscall.
4. Add test suite for `System.Contract.Call`.
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.
When providing public key as a subslice, it still can be
decoded as a valid key, thus interop will not return an error
but rather push `false` on stack. This test is about providing
invalid key, so ensure this via setting invalid prefix.
Part of #1055.
It should have `AllowStates` flag.
Also removed unreachable code: we can't have such situation when
script container is not a transaction in the scope of `CheckWitness`
method because:
1. Blocks have their own implementation of CheckWitness for
internal usage (it's (bc *Blockchain) verifyHeaderWitnesses method).
2. For the outside calls of System.Runtime.CheckWitness interop (e.g.
calls from smart-contract) script container is always a transaction.
Part of #1055.
Split methods, as they have a lot of common code. This also fixex nil
error of storageGetReadOnlyContext in case when contract does not have
storage.
Fixes#1144. It's quite simple approach, we just update balance info right
upon contract migration. It will slow down migration transactions, but it
takes about 1-2 seconds to Seek through balances at mainnet's 3.8M, so the
approach should still work good enough. The other idea was to make lazy
updates (maintaining contract migration map), but it's more complicated to
implement (and implies that a balance get might also do a write).
There also is a concern about memory usage, it can give a spike of some tens
of megabytes, but that also is considered to be acceptable.
Part of #1055.
We should check contract scripthash against the one provided in manifest
and manifest groups. We shouldn't put on stack anything after return.
And ofcourse, we mast not destroy the old contract at the end, as
`contractDestroy` removes all storage items associated with the
old contract ID (which equals to the new contract ID). We just remove
old contract state - it's enough.
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 #1133
It will help us to use big.Int to store amount of NEP5 tokens. As far as
big.Int doesn't have constant size, we shouldn't use `NEP5TransferSize`
constant anymore.
Even if the value is zero, the GAS distribution updates the balance height, so
storage item must be updated too. Fixes the followin on preview2 testnet:
block 74227: value mismatch for key ffffffff1454a6cb279fbcedc66162ad4ad5d1d910202b92743e000000000000000000000005: 1041032104809fd5002103f3210128010000 vs 1041032104809fd50021033f110128010000
They make no sense. Fixes preview2 testnet state problem:
file BlockStorage_100000/dump-block-70000.json: block 69935: state mismatch for key ffffffff1454a6cb279fbcedc66162ad4ad5d1d910202b92743e000000000000000000000005: Deleted vs Added
And fix contract create to really update the ID, eliminating this difference
in the storage (preview2 testnet):
file BlockStorage_100000/dump-block-39000.json: block 38043: key mismatch: 0c000000617373657464df4ebe92334d1fc7e64b10f1d1e33942d9905e510000000000000009 vs 00000000617373657464df4ebe92334d1fc7e64b10f1d1e33942d9905e510000000000000009
Preview2 testnet:
file BlockStorage_100000/dump-block-12000.json: block 11562: key mismatch: feffffff1454a6cb279fbcedc66162ad4ad5d1d910202b92743e000000000000000000000005 vs feffffff1431b7e7aea5131f74721e002c6a56b610885813f79e000000000000000000000005
Originally this code was written to run after transactions processing, but
after 0fa4c49735 it works in different manner.
ValidatorsCount is not initialized at block 0 with C# node (the first voter
initializes it) and until that initialization happens the standby validators
list is being returned as is without sorting.
Fixes state mismatch for the key ffffffff0e00000000000000000000000000000001 in
the first blocks.
It also affects tests as now the first validator is different and it receives
the network fees.
After block was stored it's possible to have new FeePerByte constraint,
so we should remove all transactions which do not meet this requirement.
Also caching of FeePerByte was added in order not to re-verify
transactions each time mempool needs to be updated.
MarshalJSON should be defined on structure (not pointer), as we use
structures to marshal parameters (e.g. in NotificationEvent and
Invoke of RPC result package) and never use pointers for that purpose.
Also added marshalling of nil array into `[]` instead of `null` to
follow C# implementation.
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.
C# implementation uses NEWARRAY for creating arguments.
Don't change our implementation in `emit`, because PACK is cheaper and
this script must not depend on the internal details of `emit` package anyway.
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.
It's just JSON, io.Serializable is only used for DB storage where the length
should be obtained from the stream. Fixes:
2020-06-18T22:14:10.571+0300 WARN contract invocation failed {"tx": "1ffd475a9c246495d6206cb80a9a78e9d14a433ded60cd37aa87d897655606e1", "block": 25893, "error": "error encountered at instruction 3696 (SYSCALL): failed to invoke syscall: invalid character ':' after top-level value"}
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.
And implement it for Transaction, the only user of ParameterContext for
now. Which make correct signing/verifying possible for cases when
serialization for general transmission and signing differ.
1. Remove GetScript, IsPayable, GetStorageContext.
2. Revert 82319538 related to GetStorageContext.
3. Rename Migrate to Update.
4. Move remaining to System.Contract.*.
Related #1031.
Updated System.Blockchain.GetBlock interop replaced the functionality of
the following interops:
System.Block.GetTransactions
System.Block.GetTransactionCount
Neo.Block.GetTransactions
Neo.Block.GetTransactionsCount
closes#1025
Now we put on stack stackitem.Array instead of Interop, so we're able to
use all available block properties without extra interop getters.
Removed Neo.Blockchain.GetBlock interop as we don't need it anymore.
Removed Neo.Block.GetTransaction and System.Block.GetTransaction
interops. These interops were replaced by new
System.Blockchain.GetTransactionFromBlock interop.
closes#1023
Now we put on stack stackitem.Array instead of Interop, so we don't
need old transaction-related interops anymore. Removed the following
interops:
System.Transaction.GetHash
Neo.Transaction.GetAttributes
Neo.Transaction.GetHash
Neo.Transaction.GetWitnesses
Neo.Attribute.GetData
Neo.Attribute.GetUsage
Also removed the following duplicated NEO interop:
Neo.Blockchain.GetTransaction
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.
When money is being sent usually they go away from someone's pocket, so that
there is a little less money left there. Not in our case as it turns out, we
actually were adding money both to sender and receiver which is nice, but a
bit different from usual economic's expectations.
This syscall should only work for contracts created by current transaction and
that is what is supposed to be checked here. Do so by looking at the
differences between ic.dao and original lower DAO.
Our block.Block was JSONized in a bit different fashion than result.Block in
its NextConsensus and Index fields. It's not good for notifications because
third-party clients would probably expect to see the same format. Also, using
completely different Block representation is probably making our client a bit
weaker as this representation is harder to use with other neo-go components.
So use the same approach we took for Transactions and wrap block.Block which is
to be serialized in proper way.
Fix `Script` JSONization along the way, 3.0 node wraps it within `witnesses`.
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.
`NewNEO()` and `NewGAS()` methods are trying to initialise
both `onPersist` and `incBalance` methods of NEO and GAS AFTER
nep5TokenNative is set to the VALUE of created nep5 token.
In this situation an attemmpt to call the corresponding native contracts
methods (e.g. transfer native GAS) leads to contract invocation failure,
as far as `nep5TokenNative.incBalance` method is nil.
Fixed this by initializing both `onPersist` and `incBalance` methods
before getting the value of nep5 contract.
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.
Rename THROWIFNOT to ASSERT, add ABORT opcode.
ABORT cannot be caught, but the implementation should be postponed until
exception handling is implemented.
1. Dropped `Base.ConsensusData` block field
2. Added `Block.ConsensusData` field with `Nonce` and `PrimaryIndex`
3. Removed "Neo.Header.GetConsensusData" and
"AntShares.Header.GetConsensusData" interops
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).
Our test chain is used in core,rpc and (in future) interop packages.
It is better to have all related declarations in one place to avoid
code duplication.
In NEO3 SYSCALL opcode has 4-byte ID parameter.
This commit removes support for string-based syscalls and
changes SYSCALL's parameter to be fixed 4-byte value.
When verifying transaction or block, verification script can be
a simple PUSHNULL + SYSCALL, which means that script-encontaining
entity should be verified.
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.
1. Closes#840: added Nonce field to transaction.Transaction and
removed Nonce field from transaction.MinerTx
2. Added following methods to different tx types:
- NewMinerTx()
- NewMinerTxWithNonce(...)
- NewEnrollmentTx(...)
- NewIssueTx()
- NewPublishTx(...)
- NewRegisterTx(...)
- NewStateTx(...)
in order to avoid code duplication when new transaction is created.
3. Commented out test cases where binary transaction/block are used.
These test cases marked with `TODO NEO3.0: Update binary` and need to be
updated.
4. Updated other tests
5. Added constant Nonce to GoveringTockenTx, UtilityTokenTx and genesis
block to avoid data variability. Also marked with TODO.
Which makes iterating over map stable which is important for serialization and
and even fixes occasional test failures. We use the same ordering here as
NEO 3.0 uses, but it should also be fine for NEO 2.0 because it has no
defined order.
Most of the time it's persisted into the MemoryStore or MemCachedStore, when
that's the case there is no real need to go through the Batch mechanism as it
incurs multiple copies of the data.
Importing 1.5M mainnet blocks with verification turned off, before:
real 12m39,484s
user 20m48,300s
sys 2m25,022s
After:
real 11m15,053s
user 18m2,755s
sys 2m4,162s
So it's around 10% improvement which looks good enough.
Frequently one needs to check if struct serializes/deserializes
properly. This commit implements helpers for such cases including:
1. JSON
2. io.Serializable interface
When serializing multiple accounts, cost of a buffer grow
can become significant. This commit tries to amortize it by
reusing the same buffer in a single `Persist()` call.
Fixes difference in state changes at mainnet's block 2442790 because contract
migration in b4eb2dc35226e6520ee4e09a56197dff91547b50a7f57edc82930fc18c75dffc
doesn't actually transfer the storage state, it only deletes the old one.
And add an error check just in case.
This is an append-only log which is read only during some RPCs.
It is rather slow to get it from base every time we need to append to
it. This commit stores all NEP5Transfers in batches, so that
only a last batch needs to be unmarshaled during block processing.
That's how it was intended to behave originally. One thing questionable here
is contract price (policy thing, basically) being moved to smartcontract
package, but it's probably fine for NEO 2.0 (as it won't change) and we'll
make something better for NEO 3.0.
1.5M block import time (VerifyBlocks disabled) on AMD Ryzen 5 1600/16GB/HDD,
before:
real 159m16.551s
user 69m58.279s
sys 7m34.334s
after:
real 139m41.836s
user 67m12.477s
sys 6m19.420s
12% which is even a bit more than could be expected from inputs analysis (that
has around 10% cache hits for a block-wide cache), worth doing.
This change reduces pressure on DB by doing the following things:
* not storing additional KV pair for SpentCoin
* storing Output right in the UnspentCoin, thus eliminating the need to get a
full transaction from DB
At the same time it makes UnspentCoin more fat and hot, but it should probably
worth it.
Also drop `GetUnspentCoinStateOrNew` as it shouldn't ever existed, UTXOs
can't come out of nowhere.
1.5M block import time (VerifyBlocks disabled) on AMD Ryzen 5 1600/16GB/HDD,
before:
real 302m9.895s
user 96m17.200s
sys 13m37.084s
after:
real 159m16.551s
user 69m58.279s
sys 7m34.334s
So it's almost two-fold which is a great improvement.
C# uses ToArray() or UintXXX(bytes) here which interprets hashes as they
should be interpreted (BE, although they always convert to LE when converting
to String just for the fun of it). It leads to state difference for us at
block 2025204 where even though we have the same value for the key, the key
itself differs, ours:
dd2b538e2a0c1db1ae5061c15be14f916bd1e678e512ffcda6d9499d8e7fe97ee71fd6b8004583d9afe09cc4dadbd5deb63d01e061009b7cffdaa674beae0f930ebe6085af900093e5fe56b34a5c220ccdcf6efc336fc5000000000000000000000000000000000010
theirs:
dd2b538e2a0c1db1ae5061c15be14f916bd1e67861e0013db6ded5dbdac49ce0afd9834500b8d61fe77ee97f8e9d49d9a6cdff12e5009b7cffdaa674beae0f930ebe6085af900093e5fe56b34a5c220ccdcf6efc336fc5000000000000000000000000000000000010
In this key there is a tx hash encoded
(e512ffcda6d9499d8e7fe97ee71fd6b84583d9afe09cc4dadbd5deb63d01e061 in LE used
by all the tools like neoscan).
I love Neo.
Both verification and invocation scripts need to
be unmarshaled from hex.
Also fix failing RPC tests: block contains non-pointer
`transaction.Witness` field and (*Witness).MarshalJSON method
is not called.
They have it specified right in the transaction. Unfortunately, this little
change rendered invalid our RPC test chain, but I think it became even better
after it, especially given that chain generation is a nice test by itself, so
it should be running as a regular test.
prevHash == input.PrevHash, so make less DB accesses and more real work. Fix
some bugs along the way:
* spentCoins structure may already be present in the DB when persisting TX,
there is nothing wrong with that and we shouldn't overwrite it
* it's only used for NEO and only to check for claim validity. Thus, when
processing claim tx the corresponding spentCoins should always be present
in the DB
Everywhere in this code prevHash == input.PrevHash, thus we can easily move
some common code out of the loop saving on DB accesses and
serialization/deserialization.
Implement mempool and consensus block creation policies, almost the same as
SimplePolicy plugin for C# node provides with two caveats:
* HighPriorityTxType is not configured and hardcoded to ClaimType
* BlockedAccounts are not supported
Other than that it allows us to run successfuly as testnet CN, previously our
proposals were rejected because we were proposing blocks with oversized
transactions (that are rejected by PoolTx() now).
Mainnet and testnet configuration files are updated accordingly, but privnet
is left as is with no limits.
Configuration is currently attached to the Blockchain and so is the code that
does policying, it may be moved somewhere in the future, but it works for
now.
Both are very useful outside of the core, this change also makes respective
transactions initialize with the package as they don't depend on any kind of
input and it makes no sense recreating them again and again on every use.
It wasn't sorted when all validators were elected. There is also no need to do
`Unique()` on the result because validators are distinguished by the key, so
no two registered validators can have the same key.
- Attribute should have 2 fields (usage, data)
- VOut should have 4 (5) fields (asset, value, address, n)
- Script should have 2 fields (invocation, verification)
As C# node does it. Technically it's only needed for consensus and could be
implemented in the appropriate package, but for better compatibility with C#
node we're better returning it sorted right here.
We were completely lacking ValidatorsCount that is supposed to track the
number of votes with particular count of consensus nodes which in theory can
change the number of active consensus nodes (if it ever to exceed the number
of standby validators), so we were not producing the right count and based on
that not giving the right set of validators.
Fixes#512.
Simple as that: UnregisteredAndHasNoVotes != !RegisteredAndHasVotes
Registered validators should stay in the DB, we might be in the process of
updating votes for them and that starts with subtraction.
Lack of FreeGasLimit in privnet leads to gas limit exceeding in case of transactions with small amount of GAS to be used for invoke operation (< real cost of the transaction). Solution: Fixed constraint in case when FreeGasLimit == 0. So now we are able to perform transactions in privnet with FreeGasLimit = 0 for free.
Tesnet sync failed with:
Feb 07 00:04:19 nodoka neo-go[1747]: 2020-02-07T00:04:19.838+0300 WARN blockQueue: failed adding block into the blockchain {"error": "failed to store notifications: not supported", "blockHeight": 713984, "nextIndex": 713985}
because some (not so) smart contract emitted a notification with an
InteropItem inside.
Fix mempool and chain locking
This allows us easily make 1000 Tx/s in 4-nodes privnet, fixes potential
double spends and improves mempool testing coverage.
Our mempool only contains valid verified transactions all the time, it never
has any unverified ones. Unverified pool made some sense for quick unverifying
after the new block acceptance (and gradual background reverification), but
reverification needs some non-trivial locking between blockchain and mempool
and internal mempool state locking (reverifying tx and moving it between
unverified and verified pools must be atomic). But our current reverification
is fast enough (and has all the appropriate locks), so bothering with
unverified pool makes little sense.
We not only need to remove transactions stored in the block, but also
invalidate some potential double spends caused by these transactions. Usually
new block contains a substantial number of transactions from the pool, so it's
easier to make one pass over it only keeping valid items rather than remove
them one by one and make an additional pass to recheck inputs/witnesses.
It doesn't harm as we have transactions naturally ordered by fee anyway and it
makes managing them a little easier. This also makes slices store item itself
instead of pointers to it which reduces the pressure on the memory subsystem.
They shouldn't depend on the chain state and for the same transaction they
should always produce the same result. Thus, it makes no sense recalculating
them over and over again.
Eliminate races between tx checks and adding them to the mempool, ensure the
chain doesn't change while we're working with the new tx. Ensure only one
block addition attempt could be in progress.
If blockchain is not closed, logging in defer can occur
after test has finished, which will lead to a panic with
"Log in goroutine after Test* has completed".
Error in Seek means something is terribly wrong (e.g. db was not opened) and
error drop is not the right thing to do, because caller
will continue working with the wrong view.
Turns out, our dApps use it a lot and we were going to the DB to get it which
is a useless waste of time. Technically we could also remove blockHeight here,
but not doing it at the moment as it's more involved.
It eliminates this time waste from the pprof graph, but doesn't change 1.4M ->
1.5M 100K mainnet block import test case in any noticeable way.
This solves two problems:
* adds support for shortened SYSCALL form that uses IDs (similar to #434, but
for NEO 2.0, supporting both forms), which is important for compatibility
with C# node and mainnet chain that uses it from some height
* reworks interop plugging to use callbacks rather than appending to the map,
these map mangling functions are clearly visible in the VM profiling
statistics and we want spawning a VM to be fast, so it makes sense
optimizing it. This change moves most of the work to the init() phase
making VM setup cheaper.
Caveats:
* InteropNameToID accepts `[]byte` because that's the thing we have in
SYSCALL processing and that's the most often usecase for it, it leads to
some conversions in other places but that's acceptable because those are
either tests or init()
* three getInterop functions are: `getDefaultVMInterop`, `getSystemInterop`
and `getNeoInterop`
Our 100K (1.4M->1.5M) block import time improves by ~4% with this change.
First of all, it was wrong, it was not checking for inputs really, it compared
tx hashes for some reason, second, when it did compare inputs it compared only
the PrevIndex part of them which is also wrong.
Also, there is absolutely no reason to go through GetVerifiedTransactions()
here, we don't need this copy of pointers and it can also be outdated by the
time we're to finish our check.
Before:
BenchmarkTXPerformanceTest-4
5000 485506 ns/op 65886 B/op 409 allocs/op
ok github.com/CityOfZion/neo-go/integration 3.212s
After:
enchmarkTXPerformanceTest-4
5000 371104 ns/op 44367 B/op 408 allocs/op
ok github.com/CityOfZion/neo-go/integration 2.712s
This simple change improves our BenchmarkTXPerformanceTest by 14%, just
because we don't waste time on reallocations during append().
Before:
10000 439754 ns/op 218859 B/op 428 allocs/op
ok github.com/CityOfZion/neo-go/integration 5.423s
After:
10000 369833 ns/op 87209 B/op 412 allocs/op
ok github.com/CityOfZion/neo-go/integration 4.612s
It's useless work being done before it's actually needed. These (updated with
new values) are going to be written with some kind of Put anyway, so writing
them here is just a waste of time.
We're spending a lot of time here, 100K blocks import starting at 1.4M, before
this patch:
real 4m17,748s
user 6m23,316s
sys 0m37,866s
After:
real 3m54,968s
user 5m56,547s
sys 0m39,398s
9% is quite a substantial improvement to justify this change.
Importing 100K blocks starting at 1.4M, before this patch:
real 6m0,356s
user 8m52,293s
sys 0m47,372s
After this patch:
real 4m17,748s
user 6m23,316s
sys 0m37,866s
Almost 30% better.
It's a getter function and even though it's quite fancy with its transactions
processing (for consensus operation) it shouldn't ever change the state of the
Blockchain. If we're to change anything here these changes may conflict with
the actual block processing later or may lead to broken state (if transactions
won't be approved for some reason).
go vet is not happy about them:
pkg/io/binaryReader.go:92:21: method ReadByte() byte should have signature ReadByte() (byte, error)
pkg/io/binaryWriter.go:75:21: method WriteByte(u8 byte) should have signature WriteByte(byte) error
This seriously improves the serialization/deserialization performance for
several reasons:
* no time spent in `binary` reflection
* no memory allocations being made on every read/write
* uses fast ReadBytes everywhere it's appropriate
It also makes Fixed8 Serializable just for convenience.
add dao which takes care about all CRUD operations on storage
remove blockchain state since everything is stored on change
remove storage operations from structs(entities)
move structs to entities package
This change (closely related to the neo-project/neo#1321 proposal) speeds up
1.4M mainnet blocks import by 30%. Basically, we're eliminating key decoding
for block's multisignature that has the same keys most of the time.
Things I don't like about this patch:
* yet another parameter for verifyHashAgainstScript()
* vm keys are not copied in/out
But it's rather simple and solves the problem for this particular case, so I
think it's worth it.
It makes very little sense having pointers here, these structures MUST have
some kind of key and this key is not gonna be wandering somewhere on its
own. Fixes a part of #519.
It reduces heap pressure a little for these elements as we don't have to
allocate/free them individually. And they're directly tied to transactions or
block, not being shared or anything like that, so it makes little sense for
them to be pointer-based. It only makes building transactions a little easier,
but that's obviously a minor usecase.
Before this patch on block import we could easily be spending more than 6
seconds out of 30 in Uint256 encoding for UnspentBalance, now it's completely
off the radar.
When 74590551 introduced this code we had no proper caching layer, so there
were these strange fallbacks in the code. fc0031e5 should'd removed them, but
failed to do so, so do it now and fix processing of transactions that touch
storage for the same key (address) in the same block.
Drop wif.GetVerificationScript(), drop
smartcontract.CreateSignatureRedeemScript(), add GetVerificationScript()
directly to the PublicKey and use it everywhere.
This allows easier reuse of opcodes and in some cases allows to eliminate
dependencies on the whole vm package, like in compiler that only needs opcodes
and doesn't care about VM for any other purpose.
And yes, they're opcodes because an instruction is a whole thing with
operands, that's what context.Next() returns.
Commit c80ee952a1 removed temporary store used
to contain changes of the block being processed. It's wrong in that the block
changes should be applied to the database in a single transaction so that
there wouldn't be any intermediate state observed from the outside (which is
possible now). Also, this made changes commiting persist them to the
underlying store effectively making our persist loop a no-op (and not
producing `persist completed` log lines that we love so much).
add processing of validators while block persist;
add validator structure with decoding/encoding;
add validator get from store;
add EnrollmentTX and StateTX processing;
add pubkey decode bytes, unique and contains functions;
Fixes failure to process transaction from the block when it was relayed
initially:
WARN[0788] blockQueue: failed adding block into the blockchain blockHeight=7270 error="transaction 35088916403e5cf2152e16c3bc6e0fba20c955fba38543b9fa5c50a3d3a4ace5 failed to verify: invalid transaction due to conflicts with the memory pool" nextIndex=7271
WARN[0790] blockQueue: failed adding block into the blockchain blockHeight=7270 error="transaction 35088916403e5cf2152e16c3bc6e0fba20c955fba38543b9fa5c50a3d3a4ace5 failed to verify: invalid transaction due to conflicts with the memory pool" nextIndex=7271
WARN[0790] blockQueue: failed adding block into the blockchain blockHeight=7270 error="transaction 35088916403e5cf2152e16c3bc6e0fba20c955fba38543b9fa5c50a3d3a4ace5 failed to verify: invalid transaction due to conflicts with the memory pool" nextIndex=7271
If we're to receive some 500 headers (less than `headerBatchCount`) and quit
before receiving more of them we end up with clean `bc.headerList` that will
be inited going backwards to the `targetHash`, but code path doesn't add add
the `targetHash` itself which it should do in this particular case, otherwise
we end with no genesis block hash in the list.
Missing it the following line could fail on subsequent restarts:
currHeaderHeight, currHeaderHash, err := storage.CurrentHeaderHeight(bc.store)
if the node was stopped before any headers had been received.
Extend Blockchainer with one more method to spawn a VM for test runs and use
it to run scripts. Gas consumption is not counted or limited in any way at the
moment (see #424).
NewInvocationTX() returned a version number one transaction that actually
failed to pass that version down to the invocation data which lead to
serialization/deserialization inconsistency.
VM should be responsible for code execution and in case anyone interested in additional logging or errors they could handle them like we do it iin cli.
We're about stored values here, so print those, which avoids blocking in
bc.HeaderHeight() and removes duplication between blockHeight and
persistedHeight. Fixes saving the blockchain on exit (deferred function in
Run() blocked in persist()).
Test modification was required because storeBlocks() doesn't actually save
headers and thus TestGetTransaction started to fail on persist().
If you're to sync less than 2000 headers no batched header key-value is
gonna be written into the DB and init() would panic because
bc.headerList.Len() would return 0. Use genesis block as a target in this
case.
Goreport:
neo-go/pkg/core/contract_state_test.go
Line 21: warning: "Contracto" is a misspelling of "Contraction" (misspell)
Line 64: warning: "Contracto" is a misspelling of "Contraction" (misspell)
neo-go/pkg/core/interop_neo.go
Line 420: warning: "succeedes" is a misspelling of "succeeds" (misspell)
neo-go/pkg/network/discovery.go
Line 118: warning: "succeded" is a misspelling of "succeeded" (misspell)
Line 128: warning: "successfuly" is a misspelling of "successfully" (misspell)
commit methods duplicated putSmthIntoStore functions, but have MemCachedStore
now that can easily substitute for a Batch, especially given that interop
needs something like that for its storage purposes anyway.
This adds the following verifications:
* merkleroot check
* index check
* timestamp check
* witnesses verification
VerifyWitnesses is also renamed to verifyTxWitnesses here to not confuse it
with verifyBlockWitnesse and to hide it from external access (no users at the
moment).
Linter isn't happy with our recent changes:
pkg/core/contract_state.go:109:1: receiver name cs should be consistent with previous receiver name a for ContractState
pkg/core/contract_state.go:114:1: receiver name cs should be consistent with previous receiver name a for ContractState
pkg/core/contract_state.go:119:1: receiver name cs should be consistent with previous receiver name a for ContractState
But actually `a` here most probably is a copy-paste from AssetState methods,
so fit the old code to match the new one.
Enable transaction verification for privnets and tests, testnet can't
successfuly verify block number 316711 with it enabled and mainnet stops at
105829.
