[pallets/weaver] Off-by-one <= in validate_attestation_signature rejects every honest OCW attestation

Rusdiwireng · October 2, 2026, 11:19am

Component: ghost-node @ 0162da4604d9931c16d12a0cec40dd85951368c5 — pallets/weaver/src/lib.rs:642 (validate_attestation_signature).

Description

validate_unsigned for extend_warp / weave_weft calls validate_attestation_signature, which contains:

// pallets/weaver/src/lib.rs:642-646
if T::BlockNumberProvider::current_block_number() <= attestation_block {
    return Err(InvalidTransaction::BadProof);
}

The off-chain worker builds attestations stamped with the block it is processing (AttestationContext::default().with_current_block(current_block), lib.rs:776-780, where current_block is the now passed from offchain_worker(now)). When the transaction enters the pool, the pool validates it against the current best block, which is that same now. So now <= now is always true and the pool discards the transaction with BadProof.

extend_warp and weave_weft are the only two ways oracle data (external block numbers, Merkle roots, block hashes) gets on chain. Through the normal unsigned-transaction path, neither can ever enter the pool.

The existing test suite masks this: every validate_unsigned test advances the block first (set_block_number(N+1)) before validating, so the <= branch never triggers. No test validates an attestation at the block it was created, which is the production scenario.

Impact

Total loss of bridge liveness. Without extend_warp, no external block-number consensus ever starts; without weave_weft, no external Merkle root or block hash is ever committed to LoomStates. Every bridge function (weaving plus claims via pull_thread) is dead. The only workaround is a block author manually inserting the transactions past its own pool.

Reproduction

Local test poc_w1_ocw_attestation_rejected_at_submission_block in pallets/weaver/src/poc.rs:

Set the mock chain block to 5.
Build a BlockAttestation with with_current_block(5) (exactly what the OCW does at block 5).
Sign it with an authority key, wrap it in Call::extend_warp.
Call GhostWeaver::validate_unsigned(TransactionSource::External, &call) → Err(InvalidTransaction::BadProof).
Control: set block to 6, repeat → Ok.

A second test proves the same rejection for weave_weft. Ran cargo test -p ghost-weaver --lib poc_w1: passed.

Severity: High

Reason: the pallet’s core bridge function is 100% non-functional through the normal path in production (a permanent denial-of-service against all bridge features). Not Critical because it enables no theft or unauthorized minting; it blocks everything instead.

Recommendation

Change <= to < at lib.rs:642 so only attestations for future blocks are rejected. Signature re-verification at dispatch keeps the security properties unchanged.

Verification status

VERIFIED. Mechanism confirmed line-by-line in source; PoC executed locally and passed.

VoidDragon · October 3, 2026, 10:03pm

Thank you for your report!

It would be good to reproduce the inability to register anything external on the development network before proceeding. As an external gatekeeper address, you can use the same contract on the Sepolia network that was used for testing: 0x7fec11e652f4d54e8cf11c4b52c360380332f2bd.

On this point: the OCW starts at the end of each block T. So how could the node push a value for that block when the work only begins once the block has ended? Could you please clarify your idea?

Rusdiwireng · October 4, 2026, 3:54pm

Good questions. On the timing: the OCW’s now is the block that just finished importing (block T). It builds the attestation stamped with T and hands it to the local transaction pool via SubmitTransaction while the node’s best block is still T. The pool runs validate_unsigned immediately at submission, and T::BlockNumberProvider is System in the runtimes, so current_block_number() is T, the same T the attestation is stamped with. T <= T yields BadProof, and the transaction is rejected before it ever propagates. The node never needs to push the value “during” block T; the binding constraint is pool-side validation at submission time against best block T.

So in the normal flow the rejection is deterministic, not a race: OCW submits at T, pool validates at T. (If an OCW were delayed past T+1 the check would pass, but relying on slow OCWs is not a workaround.)

On reproduction: my PoC ran against the pallet’s mock runtime (validate_unsigned at the submission block returns BadProof, and Ok once the block advances). I can reproduce the pool-level rejection end to end on a local devnet. Is there a standard devnet harness you’d like me to use, or should I stand one up from the repo’s dev setup?

On the Sepolia contract: that address is the attested data source, which is orthogonal here. The rejection happens in validate_unsigned before any attestation content is evaluated, so it bites regardless of which contract is being attested.

VoidDragon · October 5, 2026, 11:29am

This issue is really interesting. During the initial draft, I personally ran the development chain locally with Alice, Bob, Carol, and Dave. For testing purposes, we added the actual Gatekeeper contract address to watch on the Sepolia network.

The exact same check persists there - take a look.
The genesis was filled from the actual RPC for the Sepolia testnet - see here.

That’s why your reported issue is unclear: the development network successfully reached the latest Merkle root hash and registered it as LoomState. So I’m questioning it - why do you think it will break? You can run development chain on your own.

Rusdiwireng · October 6, 2026, 4:29am

Thanks for running this on the devnet, that is useful data.

You are right to question the “deterministic” framing in my report, and I want to correct it. My PoC submitted an attestation stamped T for validation at T (synchronous, mock runtime), which is the only timing that the <= check rejects. A production OCW does external RPC I/O (fetching Sepolia block data, reaching consensus across endpoints) before it calls submit_unsigned_transaction, so by the time the transaction reaches the pool the best block is usually T+1 or later, and T+1 <= T is false. That explains why your devnet reaches LoomState fine: the check passes by timing, not by correctness.

So the narrowed claim is this: the <= check makes bridge liveness timing-dependent instead of guaranteed. Any attestation submitted in the same block it is stamped with is rejected with BadProof, and there is no retry with a fresh stamp (the OCW only logs the submission error). On your devnet the RPC latency hides this; with a fast local RPC endpoint or cached responses, same-block submissions would start failing.

The fix recommendation stands (<= to <, rejecting only genuinely future-dated attestations), but the severity framing in my report was overstated. I will leave the severity call to you given the devnet evidence.

Happy to run the devnet myself to confirm the timing behavior if that would help.

VoidDragon · October 6, 2026, 9:46am

But let me slightly expand on your report.

What if there are network delays between nodes? Would that create any issues?

By the way, changing <= to < won’t help if the delay between nodes is more than one block - in that case, it will still be rejected until the nodes sync.

Rusdiwireng · October 7, 2026, 7:23am

Fair point, and you are right about the lagging-node case. Let me separate the two timing situations, because the fix addresses only one of them.

Case 1: node is in sync (best = T), attestation stamped T, submitted at T. Under <= this is rejected; under < it is accepted. This is the case my narrowed claim is about: a same-block submission is legitimate (the attestation is for the block being processed), and rejecting it makes liveness depend on OCW latency rather than protocol rules.

Case 2: node is lagging (best < T), attestation stamped T. Both <= and < reject it, and that is arguably correct: from that node’s perspective the attestation is future-dated, and it should not be validating attestations for blocks it has not seen yet. “Rejected until the nodes sync” is the right behavior there, not a defect.

So I agree the <= to < change does not help a node that is more than a block behind, but it was never meant to: the fix targets the synced-node same-block submission, which is the normal operating case. The lagging-node rejection is correct behavior under either operator.

VoidDragon · October 7, 2026, 1:41pm

That’s what I’m missing here - if possible, could you explain it more? From what I understand:

At the end of the block, a new OCW is spawned.
The OCW runs for some time.
Then the unsigned signature is sent.

How is it possible for the OCW to fall into the same T block it was started from? It looks like there’s no way it can land in the same T. What am I missing here? You keep pushing this forward, but I don’t really understand how it’s possible - especially given the results from the development chain we ran locally with 4 validators.

Rusdiwireng · October 8, 2026, 5:46am

Let me explain the exact race, because your mental model is right for the slow-RPC case and the failure needs the fast case.

offchain_worker(now) is invoked immediately after block T is imported, while the local best block is still T. The OCW then does its work: RPC calls to the external endpoints, collecting block numbers, reaching consensus. Only after that does it call submit_unsigned_transaction. The pool validates the transaction immediately at submission, against whatever the best block is at that moment.

So there are two outcomes depending on a race between OCW latency and block time:

OCW slower than block time (your devnet): by the time the transaction reaches the pool, best is T+1 or later. Attestation stamped T, check is T+1 <= T, false, accepted. This is what you observed, and it is consistent with my narrowed claim, not contradictory.
OCW faster than block time: local stub RPC, cached responses, instant-seal chain. The transaction reaches the pool while best is still T. Check is T <= T, true, rejected with BadProof. There is no retry with a fresh stamp; the OCW only logs the submission error.

That is why I corrected the framing from “deterministic” to “timing-dependent”: the check does not always bite, it bites exactly when the OCW wins the race against block production. To reproduce the failure, point the RPC at a fast local stub instead of Sepolia and watch same-block submissions get rejected.

I am not pushing for a severity here, just making sure the mechanism is precisely described. The <= to < change would make the outcome independent of that race.

VoidDragon · October 11, 2026, 2:53pm

That’s one of the most interesting discussions, in my opinion. We’re talking about a one-symbol change, and we’ve already got 10 posts about it. The only thing I’m worried about is a double-spend of the same validator signature - but it looks like that’s impossible in any case.

By the way, it seems like the whole idea of measuring by block isn’t a good one here, as we’ve highlighted before, because of possible block delays between nodes. Maybe, to avoid reuse of signatures, it makes sense to store the signatures we have in extend_warp - which should be unique for each transaction call and could easily be read during validate_unsigned. What are your thoughts on this?