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.
