061.md

Obedient Wool Canary

High

Vigilantes will experience race-conditions, which will result in processing the same checkpoints, and thus lots of wasted resources

Summary

Vigilante reporter module does not verify on babylon chain that checkpoint weren't previously submitted by another vigilante reporter. This forces each vigilante node to process each checkpoint submission on Bitcoin, from each vigilante submitter/relayer module, resulting in tons of wasted funds ( in tx fees ).

Similar issue could apply for the submitter module, when multiple vigilantes are running and submitting txs to Bitcoin, as every node just polls from Babylon and submits them - the problem is that there is no check if a checkpoint is already submitted to Bitcoin in the submitter module, this also leads to a lot of wasted tx fees.

Further details

https://github.com/babylonlabs-io/vigilante/blob/0e42d2cddd1b3e02aafd5015697666375cf1458c/reporter/utils.go#L135-L161 - here in extractCheckpoints() we process checkpoints from BTC indexed block:

func (r *Reporter) extractCheckpoints(ib *types.IndexedBlock) int {
	// for each tx, try to extract a ckpt segment from it.
	// If there is a ckpt segment, cache it to ckptCache locally
	numCkptSegs := 0

	for _, tx := range ib.Txs {
		if tx == nil {
			r.logger.Warnf("Found a nil tx in block %v", ib.BlockHash())

			continue
		}

		// cache the segment to ckptCache
@>>		ckptSeg := types.NewCkptSegment(r.CheckpointCache.Tag, r.CheckpointCache.Version, ib, tx)
@>>		if ckptSeg != nil {
			r.logger.Infof("Found a checkpoint segment in tx %v with index %d: %v", tx.Hash(), ckptSeg.Index, ckptSeg.Data)
@>>			if err := r.CheckpointCache.AddSegment(ckptSeg); err != nil {
				r.logger.Errorf("Failed to add the ckpt segment in tx %v to the ckptCache: %v", tx.Hash(), err)

				continue
			}
			numCkptSegs++
		}
	}

	return numCkptSegs
}

We just extract the checkpoint and check if it has valid structure/segments, no extensive validation.

then matchAndSubmitCheckpoints() is called in ProcessCheckpoints() , to submit the checkpoints after matching their 2 parts and generating BTCSpv proof.

func (r *Reporter) matchAndSubmitCheckpoints(signer string) int {
	var (
		proofs               []*btcctypes.BTCSpvProof
		msgInsertBTCSpvProof *btcctypes.MsgInsertBTCSpvProof
	)

	// get matched ckpt parts from the ckptCache
	// Note that Match() has ensured the checkpoints are always ordered by epoch number
	r.CheckpointCache.Match()
	numMatchedCkpts := r.CheckpointCache.NumCheckpoints()

	if numMatchedCkpts == 0 {
		r.logger.Debug("Found no matched pair of checkpoint segments in this match attempt")

		return numMatchedCkpts
	}

	// for each matched checkpoint, wrap to MsgInsertBTCSpvProof and send to Babylon
	// Note that this is a while loop that keeps popping checkpoints in the cache
	for {
		// pop the earliest checkpoint
		// if popping a nil checkpoint, then all checkpoints are popped, break the for loop
		ckpt := r.CheckpointCache.PopEarliestCheckpoint()
		if ckpt == nil {
			break
		}

		r.logger.Info("Found a matched pair of checkpoint segments!")

		// fetch the first checkpoint in cache and construct spv proof
		proofs = ckpt.MustGenSPVProofs()

		// wrap to MsgInsertBTCSpvProof
@>>		msgInsertBTCSpvProof = types.MustNewMsgInsertBTCSpvProof(signer, proofs)

		// submit the checkpoint to Babylon
@>>	res, err := r.babylonClient.InsertBTCSpvProof(context.Background(), msgInsertBTCSpvProof)
		if err != nil {
			r.logger.Errorf("Failed to submit MsgInsertBTCSpvProof with error %v", err)
			r.metrics.FailedCheckpointsCounter.Inc()

			continue
		}
		r.logger.Infof("Successfully submitted MsgInsertBTCSpvProof with response %d", res.Code)
		r.metrics.SuccessfulCheckpointsCounter.Inc()
		r.metrics.SecondsSinceLastCheckpointGauge.Set(0)
		tx1Block := ckpt.Segments[0].AssocBlock
		tx2Block := ckpt.Segments[1].AssocBlock
		r.metrics.NewReportedCheckpointGaugeVec.WithLabelValues(
			strconv.FormatUint(ckpt.Epoch, 10),
			strconv.Itoa(int(tx1Block.Height)),
			tx1Block.Txs[ckpt.Segments[0].TxIdx].Hash().String(),
			tx2Block.Txs[ckpt.Segments[1].TxIdx].Hash().String(),
		).SetToCurrentTime()
	}

	return numMatchedCkpts
}

-> We process all checkpoints from each indexed, k-deep block from BTC, no matter which vigilante submitted them.

And tx will fail during execution in x/btccheckpoint/msg_server.go

func (ms msgServer) InsertBTCSpvProof(ctx context.Context, req *types.MsgInsertBTCSpvProof) (*types.MsgInsertBTCSpvProofResponse, error) {
	// Get the SDK wrapped context
	sdkCtx := sdk.UnwrapSDKContext(ctx)

	rawSubmission, err := types.ParseSubmission(req, ms.k.GetPowLimit(), ms.k.GetExpectedTag(sdkCtx))

	if err != nil {
		return nil, types.ErrInvalidCheckpointProof.Wrap(err.Error())
	}

	submissionKey := rawSubmission.GetSubmissionKey()

@>>	if ms.k.HasSubmission(sdkCtx, submissionKey) {
@>>		return nil, types.ErrDuplicatedSubmission
	}
...

Attack Path

1. Every vigilante node submits separately checkpoints onto Bitcoin, via their submitter module, this creates many checkpoints txs on BTC.
2. Every vigilante node tracks BTC for k-deep blocks and eventually all checkpoints submitted from every vigilante node are being pooled by every reporter module in every vigilante node, and submitted as separate txs to the babylon chain.
   • 2.1 Since reporter module in each vigilante act just as relayer, it also do not verify who submitted the checkpoints extracted from the bitcoin blocks.
3. Babylon chain rejects duplicates if checkpoint is already processed, however this could leave hundreds of txs as wasted resources.
4. Over time the lost funds due to many txs submitted to the babylon chain add up. ( funds spend for the tx fees )

The attack path is also valid when submitting checkpoints to the Bitcoin network itself, as Vigilantes are not synced there, and this will just result in additional cost due to submitting multiple OP_RETURN txs on Bitcoin to store the same checkpoints. ( vigilante uses local db and no "already-submitted" checks are present in submitter's relayer.go )

Impact

Vigilante is an important off-chain module. The more vigilante we have running, the more resources will be wasted, by each vigilante, as each vigilante processes others' checkpoints submitted to BTC, this results in a lot of wasted tx fees.

Recommendation

There are ways to avoid those race-conditions/minimize wasted fees, with proper extra-checks before submitting txs, as usually read/view checks are free and they can avoid paying tx fees every time. Solutions: For reporter submitting to babylon case - query to check if checkpoint is valid, before reporter forwards the checkpoint, , to avoid paying excessive fees to babylon For submitter module submitting to bitcoin case - sync between vigilantes if a checkpoint is submitted to BTC already, to avoid paying excessive fees to bitcoin