[xxo1shine]

What does this PR do?
Recheck received block witness authorization under the Manager lock after pending state is reset, so concurrent permission changes and pending transactions cannot authorize block admission. Keep witness validation before advertisement broadcast and defer sync validation to Manager. Update the shutdown target only after witness authorization succeeds.

Why are these changes required?

This PR has been tested by:

• Unit Tests
• Manual Testing

Follow up

Extra details

[Federico2014]

[MUST] Witness authorization needs to be checked against the block’s actual parent state.

The new Manager check can reject a valid competing branch. A and B are children of P, whose witness permission authorizes K1. A contains a K1 → K2 rotation, while B is signed by K1. B can reach ingress while A is still being processed and has not yet executed its permission-update transaction. The ingress check therefore still reads K1 and correctly accepts B’s signature. A then finishes and commits before B acquires the Manager lock. B is now a fork block, and Manager.java:1270 rejects it using the canonical branch’s K2 permission, although K1 remains valid under B’s parent P. Rejection happens before Khaos admission, so switchFork() cannot perform its parent-state validation. A regression test for this sequence passes on the base and fails with this change.

There is also a related pre-existing ingress limitation: TronNetDelegate.java:397 still checks advertised blocks before their parent has necessarily been applied. A K2-signed child of a parent rotating K1 → K2 can therefore be rejected using the old K1 permission before reaching Manager.

Please validate authorization against the actual parent state, defer/retry blocks when that state is unavailable, and add regression coverage for both sequences.

[SeriousCoding789]

I agree with adding the witness-signature check inside Manager.pushBlock: checking against committed state after the pending session is reset fixes the pending-state problem. My concern is with removing it from the sync network layer. After this change the in-Manager check is the only witness-signature check on the sync path, and it runs only after work the old check used to avoid.

1. Work moves into the critical section. A sync block is only buffered on receipt and validated later on the sync handler thread. At base, a block with a bad witness signature was rejected in validBlock before processBlock. With this PR it first enters pushBlock: it raises blockWaitLock (Manager.java:1241, which makes pushTransaction callers wait), takes the Manager monitor that block production also uses (:1243), runs getVerifyTxs (:1259) and resets the pending session (:1265), so all pending transactions are re-queued and executed again. Only then is it rejected at :1270. The sender still gets a BAD_BLOCK disconnect and a 1-hour ban, but that limit is per IP, so it can be repeated from many addresses.

2. The check in front of consensus.receiveBlock becomes the only guard. consensus.receiveBlock (:1274) acts on the block's witness address (duplicate-producer detection for this node's SR keys), so it should only see blocks whose signature has been verified. Today the new check runs right before it. But the fix for the fork-state issue in the comment above (validate against the block's parent state, defer when that state is unavailable) would likely skip or defer the check for some fork blocks at this point. Without a sync-layer check, those blocks would reach consensus.receiveBlock and khaosDb.push with no witness-signature check at all.

3. Runtime exceptions are no longer wrapped. validateSignature can throw unchecked exceptions on malformed input, e.g. a witness signature shorter than 65 bytes while strict ECDSA validation is off (sanitize() only trims overlong signatures). The old network wrapper turned any exception into BLOCK_SIGN_INVALID. Inside pushBlock they now escape unwrapped, skip failProcessBlock, and are logged with a full stack trace on every attempt.

4. Known block ids (minor). BlockId covers only the raw header, and both the freshBlockId shortcut (TronNetDelegate.java:272) and the "already exist" return (Manager.java:1252-1254) run before the new check. A sync copy of a known block with a garbage signature now returns success instead of BAD_BLOCK.

Restoring the old sync check unchanged would bring back the pending-state false rejections this PR fixes; on the sync path that means banning an honest peer. So rather than reverting, I'd suggest keeping a sync-layer pre-check that does not depend on pending state:

• before processBlock, a state-free check that the witness signature has a valid length and a signer can be recovered (same strictness as validateSignature), rejected with BAD_BLOCK on failure; this cannot reject a valid block;
• if the fork-state fix defers validation for some blocks, making sure a block reaches consensus.receiveBlock only after its signature has been verified, or moving receiveBlock after the deferred validation.

It might also be worth converting runtime exceptions from validateSignature into ValidateSignatureException inside pushBlock, and adding tests for: a garbage-signature sync block with a non-empty pending pool, a short witness signature with strict ECDSA off, and a known block id carrying a garbage signature.