Skip to content

Commit fed9622

Browse files
committed
fix malleability issues in system collection
See #7749 (comment)
1 parent 7a3e7c6 commit fed9622

File tree

1 file changed

+28
-14
lines changed

1 file changed

+28
-14
lines changed

engine/execution/computation/computer/computer.go

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -314,10 +314,7 @@ func (e *blockComputer) queueSystemTransactions(
314314
systemLogger zerolog.Logger,
315315
) chan TransactionRequest {
316316
allTxs := append(executeCallbackTxs, systemTxn)
317-
// add execute callback transactions to the system collection info along to existing process transaction
318-
// TODO(7749): fix illegal mutation
319317
systemTxs := systemColection.CompleteCollection.Collection.Transactions
320-
systemColection.CompleteCollection.Collection.Transactions = append(systemTxs, allTxs...) //nolint:structwrite
321318
systemLogger = systemLogger.With().Uint32("num_txs", uint32(len(systemTxs))).Logger()
322319

323320
txQueue := make(chan TransactionRequest, len(allTxs))
@@ -493,19 +490,20 @@ func (e *blockComputer) executeSystemTransactions(
493490
Logger()
494491

495492
systemCollectionInfo := collectionInfo{
496-
blockId: block.BlockID(),
497-
blockIdStr: block.BlockID().String(),
498-
blockHeight: block.Block.Height,
499-
collectionIndex: len(rawCollections),
500-
CompleteCollection: &entity.CompleteCollection{
501-
Collection: flow.NewEmptyCollection(), // TODO(7749)
502-
},
493+
blockId: block.BlockID(),
494+
blockIdStr: block.BlockID().String(),
495+
blockHeight: block.Block.Height,
496+
collectionIndex: len(rawCollections),
497+
CompleteCollection: nil, // We do not yet know all the scheduled callbacks, so postpone construction of the collection.
503498
isSystemTransaction: true,
504499
}
505500

506501
var callbackTxs []*flow.TransactionBody
507502

508503
if e.vmCtx.ScheduleCallbacksEnabled {
504+
// We pass in the systemCollectionInfo here. However, the underlying flow.Collection object
505+
// must not be used, because it cannot represent the final state of the system collection.
506+
// To that end, the CompleteCollection is nil here, such that any attempt to access the Collection will panic.
509507
callbacks, updatedTxnIndex, err := e.executeProcessCallback(
510508
callbackCtx,
511509
systemCollectionInfo,
@@ -520,6 +518,26 @@ func (e *blockComputer) executeSystemTransactions(
520518

521519
callbackTxs = callbacks
522520
txIndex = updatedTxnIndex
521+
522+
finalCollection, err := flow.NewCollection(flow.UntrustedCollection{
523+
Transactions: append(append([]*flow.TransactionBody{e.processCallbackTxn}, callbackTxs...), e.systemTxn),
524+
})
525+
if err != nil {
526+
return err
527+
}
528+
systemCollectionInfo.CompleteCollection = &entity.CompleteCollection{
529+
Collection: finalCollection,
530+
}
531+
} else {
532+
finalCollection, err := flow.NewCollection(flow.UntrustedCollection{
533+
Transactions: []*flow.TransactionBody{e.systemTxn},
534+
})
535+
if err != nil {
536+
return err
537+
}
538+
systemCollectionInfo.CompleteCollection = &entity.CompleteCollection{
539+
Collection: finalCollection,
540+
}
523541
}
524542

525543
txQueue := e.queueSystemTransactions(
@@ -569,10 +587,6 @@ func (e *blockComputer) executeProcessCallback(
569587
txnIndex uint32,
570588
systemLogger zerolog.Logger,
571589
) ([]*flow.TransactionBody, uint32, error) {
572-
// add process callback transaction to the system collection info
573-
// TODO(7749): fix illegal mutation
574-
systemCollectionInfo.CompleteCollection.Collection.Transactions = append(systemCollectionInfo.CompleteCollection.Collection.Transactions, e.processCallbackTxn) //nolint:structwrite
575-
576590
request := newTransactionRequest(
577591
systemCollectionInfo,
578592
systemCtx,

0 commit comments

Comments
 (0)