Skip to content

Commit b59d821

Browse files
committed
fix(evm): refuse to broadcast an envelope whose anchor and delta disagree
Broadcast checked only that an envelope carried a delta at all, never that the envelope's own anchor and the anchor baked into that delta actually named the same transaction. Under the normal flow they always agree, since RequestApproval and SetupPublicParams both derive the envelope's anchor and the delta's anchor from the same value, but Broadcast has no way to know how the envelope it was actually handed was built. A mismatch here is not just a theoretical validation gap. The chain only ever looks at the delta's anchor: that is what applyStateDelta checks for replay, what the digest covers, what StateCommitted is emitted for. The local side tracks the transaction by the envelope's anchor instead, finality listeners and the ttx store are keyed on it. If the two ever diverged, the transaction would apply and commit on chain under one anchor while everything local kept waiting on a different one, and after the finality timeout wrongly report a transaction that actually succeeded as failed, the same failure shape HIGH #1 fixed, just reachable through a construction bug instead of a chain-read one. Broadcast now parses the envelope's anchor and compares it against the delta's before spending any gas, and refuses if they disagree. Signed-off-by: atharrva01 <atharvaborade568@gmail.com>
1 parent 505dc26 commit b59d821

2 files changed

Lines changed: 87 additions & 0 deletions

File tree

x/token/services/network/evm/network.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,16 @@ func (n *Network) SetRecoveryStarter(f func(ns string) error) { n.startRecovery
248248

249249
// Broadcast assembles the endorsed envelope into a signed transaction, sends it, and records the
250250
// resulting raw transaction and hash back into the envelope so the caller can follow its finality.
251+
//
252+
// It checks that the envelope's anchor and its delta's own anchor agree before spending any gas.
253+
// Under the normal flow they always do, since both are derived from the same anchor at the point an
254+
// Envelope is built (RequestApproval, SetupPublicParams), but Broadcast has no way to know how the
255+
// envelope it was actually handed got here, and a mismatch here is not hypothetical: it would mean
256+
// the local bookkeeping that tracks this transaction (finality listeners, the ttx store) is keyed on
257+
// one anchor while the chain applies and emits StateCommitted for a different one. A finality wait on
258+
// the anchor Broadcast was told about would then watch an anchor the chain never touches, and after
259+
// the timeout wrongly report a transaction that actually succeeded as failed, the same failure shape
260+
// as a persistent read error, just from a construction bug instead of a chain-connectivity one.
251261
func (n *Network) Broadcast(ctx context.Context, blob any) error {
252262
env, ok := blob.(*Envelope)
253263
if !ok {
@@ -259,6 +269,15 @@ func (n *Network) Broadcast(ctx context.Context, blob any) error {
259269
if env.Delta == nil {
260270
return errors.Errorf("evm network: envelope [%s] carries no state delta", env.Anchor)
261271
}
272+
anchor, err := keys.AnchorFromTxID(env.Anchor)
273+
if err != nil {
274+
return errors.Wrapf(err, "evm network: envelope carries an invalid anchor [%s]", env.Anchor)
275+
}
276+
if env.Delta.Anchor != anchor {
277+
return errors.Errorf(
278+
"evm network: envelope anchor [%s] does not match its delta's anchor [%x]; refusing to broadcast",
279+
env.Anchor, env.Delta.Anchor)
280+
}
262281

263282
rawTx, txHash, err := n.submitter.Submit(ctx, env.Delta, env.Endorsements)
264283
if err != nil {

x/token/services/network/evm/network_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,74 @@ func TestBroadcastRejectsBadInput(t *testing.T) {
338338
})
339339
}
340340

341+
// networkWithSubmitter returns a Network wired with a real submitter over a ready mock client, for
342+
// Broadcast tests that need to get past the submitter/delta checks to exercise what follows them.
343+
func networkWithSubmitter(t *testing.T, evm *mock.EVMClient) *Network {
344+
t.Helper()
345+
c := validConfig()
346+
c.applyDefaults()
347+
require.NoError(t, c.Validate())
348+
n, err := NewNetwork("evm-net", c, evm, nil, testSubmitter(t, evm, estimateGas()), nil)
349+
require.NoError(t, err)
350+
351+
return n
352+
}
353+
354+
// TestBroadcastRejectsMismatchedAnchor is the fix for a real gap: nothing checked that an envelope's
355+
// anchor and its delta's own anchor actually agreed before spending gas on it. Under the normal flow
356+
// they always do, since RequestApproval derives both from the same anchor, but Broadcast has no way
357+
// to know how the envelope it was actually handed was built, and the local finality tracking is keyed
358+
// on the envelope's anchor while the chain applies and emits StateCommitted for the delta's: a
359+
// mismatch here would mean waiting on an anchor the chain never touches.
360+
func TestBroadcastRejectsMismatchedAnchor(t *testing.T) {
361+
evm := readySubmitterClient()
362+
n := networkWithSubmitter(t, evm)
363+
364+
elsewhere, err := keys.AnchorFromTxID(anchorHex(0xEE))
365+
require.NoError(t, err)
366+
delta := testDelta()
367+
delta.Anchor = elsewhere
368+
369+
err = n.Broadcast(t.Context(), &Envelope{
370+
Anchor: anchorHex(0x01),
371+
Delta: delta,
372+
Endorsements: [][]byte{make([]byte, 65)},
373+
})
374+
require.Error(t, err)
375+
assert.Zero(t, evm.SendRawTransactionCallCount(), "a mismatched envelope must not be broadcast")
376+
}
377+
378+
// TestBroadcastRejectsAnUnparsableAnchor checks the envelope's own anchor is validated before it is
379+
// compared against anything, rather than producing a confusing failure further down.
380+
func TestBroadcastRejectsAnUnparsableAnchor(t *testing.T) {
381+
evm := readySubmitterClient()
382+
n := networkWithSubmitter(t, evm)
383+
384+
err := n.Broadcast(t.Context(), &Envelope{
385+
Anchor: "not-hex",
386+
Delta: testDelta(),
387+
Endorsements: [][]byte{make([]byte, 65)},
388+
})
389+
require.Error(t, err)
390+
assert.Zero(t, evm.SendRawTransactionCallCount())
391+
}
392+
393+
// TestBroadcastAcceptsAConsistentEnvelope is the positive case: an envelope whose anchor and delta
394+
// agree, as every normal caller produces, must still broadcast.
395+
func TestBroadcastAcceptsAConsistentEnvelope(t *testing.T) {
396+
evm := readySubmitterClient()
397+
n := networkWithSubmitter(t, evm)
398+
399+
delta := testDelta()
400+
err := n.Broadcast(t.Context(), &Envelope{
401+
Anchor: hex.EncodeToString(delta.Anchor[:]),
402+
Delta: delta,
403+
Endorsements: [][]byte{make([]byte, 65)},
404+
})
405+
require.NoError(t, err)
406+
assert.Equal(t, 1, evm.SendRawTransactionCallCount())
407+
}
408+
341409
// --- queries -------------------------------------------------------------------------------------
342410

343411
func TestQueryTokens(t *testing.T) {

0 commit comments

Comments
 (0)