This moves the check for validating that the number of revocations does
not exceed the maximum allowed to the checkBlockSanity function where it
more naturally belongs since it does not require access to any previous
chain context.
It should be noted that this check previously used math.MaxUint8 and was
rather implicitly limited by the fact the header uses a uint8 for the
field, so this makes the requirement more explicit by defining a
constant.
This moves the test for validating the final state specified by the
header matches the calculated final state into the
checkBlockHeaderContext function where it more naturally belongs since
it is only dependent on the header and its contextual position within
the chain.
This modifies the checkBlockHeaderSanity function to include an
additional check which ensures the final state is zero prior to stake
validation height.
It should be noted that this is technically a hard fork since before
this change it was possible to use any value for the final state prior
to stake validation height. However, no blocks on mainnet nor testnet
ever used non-zero final states for blocks in that range and they are
buried behind checkpoints in the minimum required version of the
software due to the previous consensus votes, so it isn't possible to
trigger a reorg that would fork old nodes off the network anyways, so it
is safe to retroactively apply this check.
This moves the checks for validating that votes commit to the parent of
the block they are contained in and the vote bits specified by the
header match the result of the actual votes included in the block to the
checkBlockSanity function where they more naturally belong since they do
not require access to any previous chain context.
This modifies the NextBlock function to account for changes made to the
block height or previous block hash in munge functions and update the
commitments in all votes accordingly.
This moves the test for validating the pool size specified by the header
matches the calculated size into the checkBlockHeaderContext function
where it more naturally belongs since it is only dependent on the header
and its contextual position within the chain.
This modifies the exported CheckConnectBlock function to call the
checkBlockSanity and checkBlockContext functions to ensure all
validation rules are enforced from the mining code.
Since both the mining code and tests typically work with unsolved
blocks, this also introduces a new parameter on CheckConnectBlock to
pass behavior flags which allows the caller to skip the proof of work
check.
It also modifies forceHeadReorganization to call checkBlockContext for
the same reason.
Finally, all tests and call sites are updated with the appropriate
flags and various tests are updated for the changes accordingly.
This renames the mempool.Config.RelayNonStd option to AcceptNonStd which
more accurately describes its behavior since the mempool was refactored
into a separate package.
The reasoning for this change is that the mempool is not responsible for
relaying transactions (nor should it be). Its job is to maintain a pool
of unmined transactions that are validated according to consensus and
policy configuration options which are then used to provide a source of
transactions that need to be mined.
Instead, it is the server that is responsible for relaying transactions.
While it is true that the current server code currently only relays txns
that were accepted to the mempool, this does not necessarily have to
be the case. It would be entirely possible (and perhaps even a good
idea as something do in the future), to separate the relay policy from
the mempool acceptance policy (and thus indirectly the mining policy).
This introduces two new functions to the blockchain package named
headerApprovesParent and voteBitsApproveParent and updates all call
sites in the package in order to improve the readability and more
clearly describe the intention of the code.
This modifies the SSGenBlockVotedOn function to directly copy the hash
the vote commits to into a statically sized array versus calling the
hash function and thus can remove the potential runtime error.
This is preferred because the code will fail to compile if the size
of chainhash.HashSize is every changed which is desirable because it
would mean the assumptions the code makes would be invalidated.
It also updates all callers in the repository accordingly.
This moves the check for validating that there are only ticket purchases
included in blocks before stake validation height to checkBlockSanity
where is more naturally belongs since it does not require access to any
previous chain context.
This removes a duplicate check which ensures ticket purchases pay the
required amount. It is already checked by checkProofOfStake via
checkBlockSanity.
This cleans up the checkBlockSanity and make it more consistent with
the rest of the code base.
The following is an overview of the changes:
- Use the local variable throughout
- Check the header values against the expected value instead of the
reverse
- Make the errors more consistent and verbose in some cases
- Use an int64 for the type so consensus doesn't different between
computer architecture
- Correct and add some comments
This moves the checkBlockContext function to back to validate.go next to
checkBlockHeaderContext where it belongs to help minimize the
differences between the upstream code in order to facilitate easier
syncs due to less merge conflicts as a result of superfluous changes.
This moves the test for validating the block height specified by the
header matches the height it actually connects to the chain into the
checkBlockHeaderContext function where it more naturally belongs since
it is only dependent on the header and its contextual position within
the chain.
This moves the test for validating that the vote bits specified by the
header are a specific value before reaching stake validation height into
the checkBlockHeaderSanity function where it more naturally belongs
since it is solely dependent on the header.
This changes the validation that the block does not exceed the maximum
number of ticket purchases to make use of the value committed to by the
header and thus moves the test into checkBlockHeaderSanity accordingly.
This is acceptable since the code also validates the header commitment
for the number of ticket purchases later and therefore implies
correctness.
This changes the validation that the block does not contain any votes or
revocations before stake enabled height and stake validation height to
make use of the values committed to by the header and thus moves the
test into checkBlockHeaderSanity accordingly.
It also combines the two checks to only check before stake validation
height since that always comes after stake enabled height and adds an
assertion to ensure that optimization assumption holds.
This is acceptable since the code also validates the header commitments
for the number of votes and revocations later and therefore implies
correctness.
This moves the test for validating the stake difficulty specified by the
header matches the calculated difficulty into the
checkBlockHeaderContext function where it more naturally belongs since
it is only dependent on the header and its contextual position within
the chain.
Also, while here, it updates the error message to be more consistent
with existing error messages and does not incorrectly create a rule
error from an internal error in the unlikely case the difficulty fails
to be calculated.
This changes the validation that the block does not exceed the maximum
number of votes to make use of the value committed to by the header and
thus moves the test into checkBlockHeaderSanity accordingly.
This is acceptable since the code also validates the header commitment
for the number of votes later and therefore implies correctness.
This moves the test for validating the number of votes specified by the
header is at least the required minimum into the checkBlockHeaderSanity
function where it more naturally belongs since it is solely dependent on
information the header.
Also, remove the redundant check for the same condition from
checkBlockSanity since it has now already been checked according to the
value the header commits to and therefore validating the header
commitment for the number of votes implies correctness.
This modifies the checkBlockHeaderSanity function to accept the header
as intended so that it is only performing context-free checks. Since
checking the stake difficulty of all of the ticket purchases necessarily
requires the block, that check is moved into checkBlockSanity.
Also, while here, improve the comments on the {c,C}checkProofOfStake
functions. Note that having comments on an exported function that refer
to an unexported one is not very helpful since documentation generation
tools don't see unexported funcs.
This corrects the generalized reorg logic in reorganizeChain for the
case when there are only nodes being detached since the fork block would
be nil in that case.
It should be noted that nothing currently calls this code under that
scenario so this case would never be hit, but that could change in the
future, so the logic needs to account for it.
This makes the semantics of block fetching much more explicit by
introducing a new function named fetchMainChainBlockByHash which only
attempts to the load a block from the main chain block cache and then
falls back to the database. While currently only blocks in the main
chain are in the database, the intention is for future commits to
allow the database to house side chain blocks as well, so having clearly
defined semantics will help make that transition easier to verify.
While here, it also renames {f,F}etchBlockFromHash to {f,F}etchBlockByHash
to be consistent with the naming used by the other existing functions
and updates the comments to better describe the function semantics.
This modifies the block connection code paths to accept the parent block
in addition to the block as a parameter versus loading it on demand in
the called functions. This makes the dependency more explicit and also
ensures it doesn't potentially get loaded multiple times in the reorg
case.
This generalizes and optimizes the reorganize chain function to take
advantage of the linear structure of the nodes being attached and
detached in order to avoid reloading the parent blocks as well as allow
it to rewind the chain in the case no attach nodes are provided or
extend the chain in the case no detach nodes are provided.
It also adds several assertions to ensure the assumptions about the
state hold and generally cleans up the function for code consistency.
This removes the DumpBlockChain which is leftover from earlier
development. It will still be in commit history if it's ever needed
again, but it doesn't belong in production code.
This modifies the BestState struct in the blockchain package to store
hashes directly instead of pointers to them and updates callers to deal
with the API change in the exported BestState struct.
In general, the preferred approach for hashes moving forward is to store
hash values in complex data structures, particularly those that will be
used for cache entries, and accept pointers to hashes in arguments to
functions.
Some of the reasoning behind making this change is:
- It is generally preferred to avoid storing pointers to data in cache
objects since doing so can easily lead to storing interior pointers into
other structs that then can't be GC'd
- Keeping the hash values directly in the structs provides better
cache locality
This adds several tests for the CalcPastMedianTime function including
when there are less than the usual number of blocks which happens near
the beginning of the chain.
This refactors the block index logic into a separate struct and
introduces an individual lock for it so it can be queried independent of
the chain lock.
It also modifies the `newBlockNode` function to accept nil for the
ticket spend information parameter and updates all of the test code that
doesn't require it to use nil.
The current serialized "block index" is actually a main chain index, so
this renames the related functions to correctly denote it's a main chain
index and updates the comments accordingly.
This is being done since future work intends to actually serialize the
entire block index and renaming this now will make the introduction of
full block index code more clear.
This modifies the IsSStx, IsSSGen, and IsSSRtx functions to only return
a bool and introduces CheckSStx, CheckSSGen, and CheckSSRtx to return
the actual error as needed by consensus.
This is being done because "is" functions are much nicer to use when
they don't return an error and the callers that use them almost never
care why they aren't of the type, they just want to determine if they
are. In the few cases where the caller does care, they can use of the
new check functions.
While here, also update the comments to call out what the more common
names for the transaction types are and to add comments to the test
functions for consistency.
Finally, it updates all callers in the repo accordingly.
Considering that there is no method for getseed and also that your seed should be saved at wallet creation it seems best to get rid of getseed type and instance function.
This modifies the block node structure to include extra fields needed to
be able to reconstruct the block header from a node, and exposes a new
function from chain to fetch the block headers which takes advantage of
the new functionality to reconstruct the headers from memory when
possible. Finally, it updates both the p2p and RPC servers to make use
of the new function.
This is useful since many of the block header fields need to be kept in
order to form the block index anyways and storing the extra fields means
the database does not have to be consulted when headers are requested if
the associated node is still in memory.
This modifies the block node structure to include only the specifically
used fields, some of which in a more compact format, as opposed to
copying the entire header and updates all code and tests accordingly.
Not only is this a more efficient approach that helps pave the way for
future optimizations, it is also consistent with the upstream code which
helps minimize the differences to facilitate easier syncs due to less
merge conflicts.
In particular, since the merkle and stake roots, number of revocations,
size, nonce, and extradata fields aren't used currently, they are no
longer copied into the block node. Also, the block node already had a
height field, which is also in the header, so this change also removes
that duplication.
Another change is that the block node now stores the timestamp as an
int64 unix-style timestamp which is only 8 bytes versus the old
timestamp that was in the header which is a time.Time and thus 24 bytes.
It should be noted that future optimizations will very likely end up
adding most of the omitted header fields back to the block node as
individual fields so the headers can be efficiently reconstructed from
memory, however, these changes are still beneficial due to the ability
to decouple the block node storage format from the header struct which
allows more compact representations and reording of the fields for
optimal struct packing.
Ultimately, the need for the parent hash can also be removed, which will
save an additional 32 bytes which would not be possible without this
decoupling.
