Repository navigation
Conversation
Track permission-update owners when blocks are rolled back so cached transaction verification is invalidated. Clear transaction results before caching relay messages and omit full hash lists when transaction inventory is rate limited.
Release the global sync request on every block processing failure so another peer can supply the block without waiting for cache expiration. Disconnect only the sender and preserve other peers' queued blocks and sync progress. Reject advertised blocks whose witness is outside the active set instead of silently dropping them. Keep advertisement recovery on the sync path and retain log-only handling for failures during advertised block application. Unify block application errors as BAD_BLOCK while retaining their causes. Log P2P failure details without a stack trace and keep stack traces for unexpected sync failures.
Clear results only on the transaction copy cached for relay. Keep the original capsule unchanged so broadcasting cannot erase pending execution results or race with block assembly.
Reject transactions with a contract count other than one before relaying received blocks. Apply shared timestamp, transaction structure, Merkle root and witness signature checks to both advertised and sync blocks, while keeping current active-witness checks specific to advertised blocks. Validate Merkle roots at network admission and remove the redundant Manager check and capsule validation cache. Keep the existing signature failure types, messages and causes.
Merge upstream release_v4.8.3 through d7df62b. Preserve shared block prevalidation before propagation and sync processing while adopting the release inactive-witness relay guard and repeated-block disconnect policy.
| "time:" + time + ",block time:" + block.getTimeStamp()); | ||
| } | ||
| for (Transaction transaction : block.getInstance().getTransactionsList()) { | ||
| int contractCount = transaction.getRawData().getContractCount(); |
There was a problem hiding this comment.
[SHOULD] Please also check each transaction’s full serialized size here, rejecting transactions smaller than 65 bytes or larger than Constant.TRANSACTION_MAX_BYTE_SIZE before propagation.
Merkle leaves and internal nodes use the same hash function without domain separation. A 64-byte transaction can match the concatenation of two 32-byte child hashes, allowing different transaction lists to share the same Merkle root. Checking contract count alone does not rule this out.
Please add boundary tests and verify compatibility with historical blocks and shielded transactions.
There was a problem hiding this comment.
Implemented in 9b3ab87. MIN_TRANSACTION_SIZE = 80 now rejects transactions whose full Transaction.getSerializedSize() is below 80 bytes, before Merkle validation for both advertised and sync blocks.
Tests cover 64/79-byte rejection, 80/81-byte acceptance, signatures and ret counting toward the full size, a synthetic shielded transaction without an outer signature, and rejection before broadcast, fast-forwarding, or sync processing. All 62 targeted tests, the build with tests excluded, and Checkstyle passed.
This change adds the lower bound only; upper-bound validation is unchanged. A full historical-chain replay has not been performed.
Check the full serialized transaction size before Merkle validation for advertised and sync blocks. Exclude short transaction encodings from the 64-byte internal-node hash input domain.
|
|
||
| /** | ||
| * save a block. | ||
| * Save a block. Untrusted callers must first use network prevalidation to check its timestamp, |
There was a problem hiding this comment.
[DISCUSS] Confirm the Merkle-validation boundary for future callers
This removes the Merkle-root check from pushBlock and makes network prevalidation a caller prerequisite. The current advertised and sync paths both call validBlock, so I did not find a current bypass; however, a future direct caller that misses this Javadoc contract would no longer get the previous defense-in-depth check.
Suggestion: Keep the prevalidation requirement explicit for all new entry points, and consider retaining a core Merkle check or introducing a validated-block abstraction if that caller contract cannot be enforced.
There was a problem hiding this comment.
This is an intentional change in validation responsibility: the advertised-block path calls validAdvBlock -> validBlock, and the sync path calls validBlock directly before entering pushBlock. The duplicate Merkle validation in the core layer was therefore removed.
I agree with the reminder about future callers. The Javadoc for pushBlock explicitly requires untrusted blocks to undergo prevalidation, and new entry points must respect that prerequisite as well. No further code changes are planned for this comment.
Rely on BlockMsgHandler to check timestamps for advertised and sync blocks. Remove the duplicate one-second wall-clock rejection from shared block validation.
What does this PR do?
Why are these changes required?
This PR has been tested by:
Follow up
Extra details