[release-v2.1] multi: Main module backports. - #3770
Merged
Merged
Conversation
jholdstock
approved these changes
Aug 18, 2026
davecgh
force-pushed
the
rel121_main_backports
branch
from
August 18, 2026 12:43
995a2a8 to
d3a0de6
Compare
This updates the 2.1 release branch to use the latest version of the blockchain module which includes an update to the full block tests. In particular, the following updated module version is used: - github.com/decred/dcrd/blockchain@v5.1.1
The early null output check for the ticket input of revocations was inadvertently removed when the automatic revocations agenda was introduced. This restores that check. Not doing the early check doesn't cause any real issue because later checks will fail on attempting to lookup a null output anyway, but it is always ideal to fail as early as possible to avoid additional unnecessary work.
The required method for requesting and sending the initial state was changed to getinitstate and initstate in protocol version 8. However, that wasn't strictly enforced at the time to ensure any straggling implementations had a chance to catch up. Now that there have been multiple protocol versions since that time and new consensus changes that would fork any old nodes from the network anyway, this adds hard enforcement of that rule. Peers are now banned for sending the old getminingstate/miningstate messages when they have negotiated to at least protocol version 8.
The expected protocol for the initial state messages is that only a single request and response will be sent per connection. All honest peers on the network follow this rule, however it is not strictly enforced. This adds logic to strictly enforce that rule by banning any peers that violate it.
This applies the same request limits to manual requests as are applied to all of the netsync internal automatic request logic. The limiting mechanism this adds for manually requested transactions is intentionally slightly different than what is applied to the automatic request logic. In particular, the new additional manual request limiting only requests up to the max possible and ignores the rest. This distinction is made because the automatic code is more geared towards recency and has a variety of other logic that ultimately places stricter controls on the requests. On the other hand, manual requests are biased toward immediacy and have a larger theoretical attack surface.
Currently, the known blocks for each peer are only updated when receiving and sending inv announcements for blocks and when receiving the blocks themselves. That historically covered all relevant cases since syncing was purely done via getblocks and the resulting inv announcements. However, block announcements are now done via header announcements instead and that path does not currently add the associated block as known inventory when it sees the header. This ultimately leads to some unnecessary extra work that gets thrown away. This resolves that issue by updating the logic that processes announced headers to add the associated block to the known inventory accordingly.
This performs some light cleanup of the checkProofOfStake function to make it more consistent with the rest of the code make the error messages more accurate and useful.
Currently, all overflow detection is done inline in multiple places throughout the blockchain code. It would be more ergonomic, consistent, and less error prone to instead use well-tested funcs dedicated to that purpose. To pave the way, this adds two new functions for adding arguments with a returned flag that indicates whether the result is safe to use (that is no overflow or underflow occurred). One variant is for unsigned ints (uint16, uint32, and uint64) and the other is for signed ints (int16, int32, and int64). It only introduces the funcs and does not modify any code to use them.
This adds comprehensive tests for the new addUnsigned and addSigned funcs for all supported types.
Consensus code should ideally always use types of a specific size so there is no possibility of divergent behavior when compiled on different architectures due to differing upper bounds. Further, unsigned types for values that can never be negative should always be preferred. The current signature operations counting code does not adhere to that practice and uses plain ints which means the maximum possible value technically changes depending on the architecture. While this is not currently an issue due to a combination of various limits making it impossible to get anywhere near the limits of the smallest supported architecture (32-bit) and overflow detection, these types of hidden assumptions can easily lead to bugs over time as new features are introduced. This remedies that by modifying the consensus level signature operation counting to use an fixed unsigned 32-bit integer instead of an architecture dependent signed integer. Ideally, the return types on the underlying counting funcs in the script engine would be updated to match and avoid the need to cast, but that would require a major API version bump since it is a public module, so this limits the changes to the internal blockchain, mempool, and mining packages. While here, it also switches to the new consolidated add funcs with cleaner overflow detection versus the current more ad-hoc inline detection. Note that there is no risk of consensus divergence due to the aforementioned impossibility of hitting the conditions with the current combination of parameters and limits.
The requirement that treasurybases have zero fee is currently indirectly enforced by ensuring the input sum is the required subsidy and that the total of all fees paid in the stake tree do not exceed the input sum. While that doesn't cause any real issues, it's not very clear and it ideally should be done in the per-transaction input checks so it is consistent with all other transaction types. This modifes CheckTransactionInputs to apply the additional input versus output sum check to apply to treasurybase transactions as well.
Currently, enforcement that treasury spends commit to the amount they are spending in the first output and that the amount matches the value specified as the input amount in the first input happen when blocks are connected. The check is not dependent on the current treasury balance or anything that would require it to be limited to block connection only. It should ideally be in the per-transaction input checks so that it is also enforced early when a treasury spend is added to the mempool. To accomplish that, this moves that check, along with a couple of other additional sanity checks, into a separate method dedicated to checking treasury spend inputs so that it is more consistent with the other stake transaction type handling and invokes that method from CheckTransactionInputs. It also moves the other treasury spend checks after the call that checks and connects transactions in the stake tree to ensure the commitment is still verified prior to the other checks that depend on it.
This adds a couple new tests to help ensure the treasurybase overall amount sum semantics are correct.
The current code recalculates the total stake tree fees via a separate getStakeTreeFees function in order to pass them to the regular tree transaction and connection checks so they are accumulated as part of the total overall fees. This behavior is correct, but the total fees are already calculated when checking and connecting the stake tree transactions just before that, so there is no reason to calculate them again when they can simply be returned to the caller. This modifies checkTransactionsAndConnect accordingly and removes the separate method which is no longer necessary.
This updates the transaction input and fee summing code to make use of the new consolidated add funcs with cleaner overflow detection. It also consolidates the input summing for all transaction types into a new closure instead of repeating the logic multiple times throughout the input checks function. Consolidating it makes it more readable and less error prone. Finally, while here, it consolidates, cleans up and slightly optimizes the input sum handling for the transaction types that do not have normal inputs. Namely, first the stakebase summing is changes to use the input value field instead of recalculating the subsidy to make it consistent with the treasurybase and treasury spend cases. This is safe because the code earlier in the function ensures the values matches the expected amounts. Second, it combines the checks for all three types since that change makes the checks for them identical.
This modifies the code that populates the current utxo view with in-flight transations to add only the specifically referenced output of that transaction instead of all of them. This results in reducing the number of adds for such cases from O(n) to O(1). While the constant factor of n is fairly limited overall, it still is doing more work than is needed, particularly if a transaction has a lot of references.
This adds an additional defensive check to ensure stake transactions can never spend inputs from the same block they are mined in. Unfortunately, due to a separate bug that has since been fixed, there are now some existing violations in the main chain that have to be special cased to ensure the entire chain history can still be validated. In practice, this check is not possible to hit, but, it is much safer to explicitly disallow the state regardless.
davecgh
force-pushed
the
rel121_main_backports
branch
from
August 18, 2026 13:24
d3a0de6 to
851ca16
Compare
jrick
approved these changes
Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a backport of the following PRs to the 2.1 release branch: