Handle & dismiss ineffective refunds.

server/provider/scheduledMiniblocks.go

@@ -17,157 +17,186 @@ func (provider *networkProvider) simplifyBlockWithScheduledTransactions(block *a
 		return err
 	}
 
+	reportScheduledTransactions(block)
 	doSimplifyBlockWithScheduledTransactions(previousBlock, block, nextBlock)
-	deduplicatePreviouslyAppearingContractResultsInReceipts(previousBlock, block)
 
 	return nil
 }
 
-func doSimplifyBlockWithScheduledTransactions(previousBlock *api.Block, block *api.Block, nextBlock *api.Block) {
-	// Discard "processed" miniblocks in block N, since they already produced effects in N-1
-	removeProcessedMiniblocksOfBlock(block)
-
-	// Move "processed" miniblocks from N+1 to N
-	processedMiniblocksInNextBlock := findProcessedMiniblocks(nextBlock)
-	appendMiniblocksToBlock(block, processedMiniblocksInNextBlock)
+func reportScheduledTransactions(block *api.Block) {
+	numScheduled := 0
+	numProcessed := 0
+	numInvalid := 0
 
-	// Build an artificial miniblock holding the "invalid" transactions that produced their effects in block N,
-	// and replace the existing (one or two "invalid" miniblocks).
-	invalidTxs := gatherInvalidTransactions(previousBlock, block, nextBlock)
-	invalidMiniblock := &api.MiniBlock{
-		Type:         dataBlock.InvalidBlock.String(),
-		Transactions: invalidTxs,
+	for _, miniblock := range block.MiniBlocks {
+		if miniblock.ProcessingType == dataBlock.Scheduled.String() {
+			numScheduled += len(miniblock.Transactions)
+		} else if miniblock.ProcessingType == dataBlock.Processed.String() {
+			numProcessed += len(miniblock.Transactions)
+		} else if miniblock.Type == dataBlock.InvalidBlock.String() {
+			numInvalid += len(miniblock.Transactions)
+		}
 	}
-	removeInvalidMiniblocks(block)
 
-	if len(invalidMiniblock.Transactions) > 0 {
-		appendMiniblocksToBlock(block, []*api.MiniBlock{invalidMiniblock})
+	if numScheduled > 0 || numProcessed > 0 {
+		log.Info("reportScheduledTransactions()", "scheduled", numScheduled, "processed", numProcessed, "invalid", numInvalid, "block", block.Nonce)
 	}
-
-	// Discard "scheduled" miniblocks of N, since we've already brought the "processed" ones from N+1,
-	// and also handled the "invalid" ones.
-	removeScheduledMiniblocks(block)
 }
 
-func removeProcessedMiniblocksOfBlock(block *api.Block) {
-	removeMiniblocksFromBlock(block, func(miniblock *api.MiniBlock) bool {
-		return miniblock.ProcessingType == dataBlock.Processed.String()
-	})
+func doSimplifyBlockWithScheduledTransactions(previousBlock *api.Block, block *api.Block, nextBlock *api.Block) {
+	txs := gatherEffectiveTransactions(block.Shard, previousBlock, block, nextBlock)
+	receipts := gatherAllReceipts(block)
+
+	block.MiniBlocks = []*api.MiniBlock{
+		{
+			Type:         "Artificial",
+			Transactions: txs,
+		},
+		{
+			Type:     "Artificial",
+			Receipts: receipts,
+		},
+	}
 }
 
-func removeScheduledMiniblocks(block *api.Block) {
-	removeMiniblocksFromBlock(block, func(miniblock *api.MiniBlock) bool {
-		hasProcessingTypeScheduled := miniblock.ProcessingType == dataBlock.Scheduled.String()
-		hasConstructionStateNotFinal := miniblock.ConstructionState != dataBlock.Final.String()
-		shouldRemove := hasProcessingTypeScheduled && hasConstructionStateNotFinal
-		return shouldRemove
-	})
-}
+func gatherEffectiveTransactions(selfShard uint32, previousBlock *api.Block, currentBlock *api.Block, nextBlock *api.Block) []*transaction.ApiTransactionResult {
+	txsInCurrentBlock := gatherAllTransactions(currentBlock)
 
-func removeInvalidMiniblocks(block *api.Block) {
-	removeMiniblocksFromBlock(block, func(miniblock *api.MiniBlock) bool {
-		return miniblock.Type == dataBlock.InvalidBlock.String()
-	})
-}
+	scheduledTxsInPreviousBlock := gatherScheduledTransactions(previousBlock)
+	scheduledTxsInCurrentBlock := gatherScheduledTransactions(currentBlock)
 
-func gatherInvalidTransactions(previousBlock *api.Block, block *api.Block, nextBlock *api.Block) []*transaction.ApiTransactionResult {
-	// Find "invalid" transactions that are "final" in N
-	invalidTxsInBlock := findInvalidTransactions(block)
-	// If also present in N-1, discard them
-	scheduledTxsHashesPreviousBlock := findScheduledTransactionsHashes(previousBlock)
-	invalidTxsInBlock = discardTransactions(invalidTxsInBlock, scheduledTxsHashesPreviousBlock)
+	if len(scheduledTxsInPreviousBlock) == 0 && len(scheduledTxsInCurrentBlock) == 0 {
+		return txsInCurrentBlock
+	}
 
-	// Find "invalid" transactions in N+1 that are "scheduled" in N
-	invalidTxsInNextBlock := findInvalidTransactions(nextBlock)
-	scheduledTxsHashesInBlock := findScheduledTransactionsHashes(block)
-	invalidTxsScheduledInBlock := filterTransactions(invalidTxsInNextBlock, scheduledTxsHashesInBlock)
+	var previouslyExecutedResults []*transaction.ApiTransactionResult
+	var currentlyExecutedResults []*transaction.ApiTransactionResult
 
-	// Duplication might occur, since a block can contain two "invalid" miniblocks,
-	// one added to block body, one saved in the receipts unit (at times, they have different content, different hashes).
-	invalidTxs := append(invalidTxsInBlock, invalidTxsScheduledInBlock...)
-	invalidTxs = deduplicateTransactions(invalidTxs)
+	if len(scheduledTxsInPreviousBlock) > 0 {
+		previouslyExecutedResults = findImmediatelyExecutingContractResults(selfShard, scheduledTxsInPreviousBlock, txsInCurrentBlock)
+	}
+	if len(scheduledTxsInCurrentBlock) > 0 {
+		txsInNextBlock := gatherAllTransactions(nextBlock)
+		currentlyExecutedResults = findImmediatelyExecutingContractResults(selfShard, scheduledTxsInCurrentBlock, txsInNextBlock)
+	}
 
-	return invalidTxs
-}
+	// effectiveTxs
+	//	= txsInCurrentBlock
+	//	- txsInPreviousBlock (excludes transactions in "processed" miniblocks, for example)
+	//	- previouslyExecutedResults
+	//	+ currentlyExecutedResults
 
-func findScheduledTransactionsHashes(block *api.Block) map[string]struct{} {
-	txs := make(map[string]struct{})
+	effectiveTxs := make([]*transaction.ApiTransactionResult, 0)
+	effectiveTxsByHash := make(map[string]*transaction.ApiTransactionResult)
 
-	for _, miniblock := range block.MiniBlocks {
-		hasProcessingTypeScheduled := miniblock.ProcessingType == dataBlock.Scheduled.String()
-		hasConstructionStateNotFinal := miniblock.ConstructionState != dataBlock.Final.String()
-		shouldAccumulateTxs := hasProcessingTypeScheduled && hasConstructionStateNotFinal
-
-		if shouldAccumulateTxs {
-			for _, tx := range miniblock.Transactions {
-				txs[tx.Hash] = struct{}{}
-			}
-		}
+	for _, tx := range txsInCurrentBlock {
+		effectiveTxsByHash[tx.Hash] = tx
 	}
 
-	return txs
-}
+	if len(scheduledTxsInPreviousBlock) > 0 {
+		txsInPreviousBlock := gatherAllTransactions(previousBlock)
 
-func findProcessedMiniblocks(block *api.Block) []*api.MiniBlock {
-	foundMiniblocks := make([]*api.MiniBlock, 0, len(block.MiniBlocks))
+		for _, tx := range txsInPreviousBlock {
+			delete(effectiveTxsByHash, tx.Hash)
+		}
 
-	for _, miniblock := range block.MiniBlocks {
-		if miniblock.ProcessingType == dataBlock.Processed.String() {
-			foundMiniblocks = append(foundMiniblocks, miniblock)
+		for _, tx := range previouslyExecutedResults {
+			delete(effectiveTxsByHash, tx.Hash)
 		}
 	}
 
-	return foundMiniblocks
+	if len(scheduledTxsInCurrentBlock) > 0 {
+		for _, tx := range currentlyExecutedResults {
+			effectiveTxsByHash[tx.Hash] = tx
+		}
+	}
+
+	for _, tx := range effectiveTxsByHash {
+		effectiveTxs = append(effectiveTxs, tx)
+	}
+
+	return effectiveTxs
 }
 
-func findInvalidTransactions(block *api.Block) []*transaction.ApiTransactionResult {
-	invalidTxs := make([]*transaction.ApiTransactionResult, 0)
+func findImmediatelyExecutingContractResults(
+	selfShard uint32,
+	transactions []*transaction.ApiTransactionResult,
+	maybeContractResults []*transaction.ApiTransactionResult,
+) []*transaction.ApiTransactionResult {
+	immediateleyExecutingContractResults := make([]*transaction.ApiTransactionResult, 0)
+	nextContractResultsByHash := make(map[string][]*transaction.ApiTransactionResult)
 
-	for _, miniblock := range block.MiniBlocks {
-		if miniblock.Type == dataBlock.InvalidBlock.String() {
-			for _, tx := range miniblock.Transactions {
-				invalidTxs = append(invalidTxs, tx)
-			}
-		}
+	for _, item := range maybeContractResults {
+		nextContractResultsByHash[item.PreviousTransactionHash] = append(nextContractResultsByHash[item.PreviousTransactionHash], item)
 	}
 
-	return invalidTxs
+	for _, tx := range transactions {
+		immediateleyExecutingContractResultsPart := findImmediatelyExecutingContractResultsOfTransaction(selfShard, tx, nextContractResultsByHash)
+		immediateleyExecutingContractResults = append(immediateleyExecutingContractResults, immediateleyExecutingContractResultsPart...)
+	}
+
+	return immediateleyExecutingContractResults
 }
 
-// Sometimes, an invalid transaction processed in a scheduled miniblock
-// might have its smart contract result (if any) saved in the receipts unit of both blocks N and N+1.
-// This function ignores the duplicate entries in block N.
-func deduplicatePreviouslyAppearingContractResultsInReceipts(previousBlock *api.Block, block *api.Block) {
-	previouslyAppearing := findContractResultsInReceipts(previousBlock)
-	removeContractResultsInReceipts(block, previouslyAppearing)
+func findImmediatelyExecutingContractResultsOfTransaction(
+	selfShard uint32,
+	tx *transaction.ApiTransactionResult,
+	nextContractResultsByHash map[string][]*transaction.ApiTransactionResult,
+) []*transaction.ApiTransactionResult {
+	immediatelyExecutingContractResults := make([]*transaction.ApiTransactionResult, 0)
+
+	for _, nextContractResult := range nextContractResultsByHash[tx.Hash] {
+		// Not immediately executing.
+		if nextContractResult.SourceShard != selfShard {
+			continue
+		}
+
+		immediatelyExecutingContractResults = append(immediatelyExecutingContractResults, nextContractResult)
+		// Recursive call:
+		immediatelyExecutingContractResultsPart := findImmediatelyExecutingContractResultsOfTransaction(selfShard, nextContractResult, nextContractResultsByHash)
+		immediatelyExecutingContractResults = append(immediatelyExecutingContractResults, immediatelyExecutingContractResultsPart...)
+	}
+
+	return immediatelyExecutingContractResults
 }
 
-func findContractResultsInReceipts(block *api.Block) map[string]struct{} {
-	txs := make(map[string]struct{})
+func gatherScheduledTransactions(block *api.Block) []*transaction.ApiTransactionResult {
+	scheduledTxs := make([]*transaction.ApiTransactionResult, 0)
 
 	for _, miniblock := range block.MiniBlocks {
-		if !isContractResultsMiniblockInReceipts(miniblock) {
+		isScheduled := miniblock.ProcessingType == dataBlock.Scheduled.String()
+		if !isScheduled {
 			continue
 		}
 
 		for _, tx := range miniblock.Transactions {
-			txs[tx.Hash] = struct{}{}
+			scheduledTxs = append(scheduledTxs, tx)
 		}
 	}
 
-	return txs
+	return scheduledTxs
 }
 
-func removeContractResultsInReceipts(block *api.Block, txsHashes map[string]struct{}) {
+func gatherAllTransactions(block *api.Block) []*transaction.ApiTransactionResult {
+	txs := make([]*transaction.ApiTransactionResult, 0)
+
 	for _, miniblock := range block.MiniBlocks {
-		if !isContractResultsMiniblockInReceipts(miniblock) {
-			continue
+		for _, tx := range miniblock.Transactions {
+			txs = append(txs, tx)
 		}
-
-		miniblock.Transactions = discardTransactions(miniblock.Transactions, txsHashes)
 	}
+
+	return txs
 }
 
-func isContractResultsMiniblockInReceipts(miniblock *api.MiniBlock) bool {
-	return miniblock.Type == dataBlock.SmartContractResultBlock.String() && miniblock.IsFromReceiptsStorage
+func gatherAllReceipts(block *api.Block) []*transaction.ApiReceipt {
+	receipts := make([]*transaction.ApiReceipt, 0)
+
+	for _, miniblock := range block.MiniBlocks {
+		for _, receipt := range miniblock.Receipts {
+			receipts = append(receipts, receipt)
+		}
+	}
+
+	return receipts
 }

server/services/networkProviderExtension.go

@@ -78,6 +78,10 @@ func (extension *networkProviderExtension) getGenesisBlockIdentifier() *types.Bl
 	return blockSummaryToIdentifier(summary)
 }
 
+func (extension *networkProviderExtension) isContractAddress(address string) bool {
+	return !extension.isUserAddress(address)
+}
+
 func (extension *networkProviderExtension) isUserAddress(address string) bool {
 	pubKey, err := extension.provider.ConvertAddressToPubKey(address)
 	if err != nil {

server/services/transactionsFeaturesDetector.go

@@ -5,14 +5,16 @@ import (
 )
 
 type transactionsFeaturesDetector struct {
-	networkProvider  NetworkProvider
-	eventsController *transactionEventsController
+	networkProvider          NetworkProvider
+	networkProviderExtension *networkProviderExtension
+	eventsController         *transactionEventsController
 }
 
 func newTransactionsFeaturesDetector(provider NetworkProvider) *transactionsFeaturesDetector {
 	return &transactionsFeaturesDetector{
-		networkProvider:  provider,
-		eventsController: newTransactionEventsController(provider),
+		networkProvider:          provider,
+		networkProviderExtension: newNetworkProviderExtension(provider),
+		eventsController:         newTransactionEventsController(provider),
 	}
 }
 
@@ -106,3 +108,9 @@ func (detector *transactionsFeaturesDetector) isContractCallWithSignalError(tx *
 func (detector *transactionsFeaturesDetector) isIntrashard(tx *transaction.ApiTransactionResult) bool {
 	return tx.SourceShard == tx.DestinationShard
 }
+
+// isSmartContractResultIneffectiveRefund detects smart contract results that are ineffective refunds.
+// Also see: https://console.cloud.google.com/bigquery?sq=667383445384:de7a5f3f172f4b50a9aaed353ef79839
+func (detector *transactionsFeaturesDetector) isSmartContractResultIneffectiveRefund(scr *transaction.ApiTransactionResult) bool {
+	return scr.IsRefund && scr.Sender == scr.Receiver && detector.networkProviderExtension.isContractAddress(scr.Sender)
+}

server/services/transactionsFeaturesDetector_test.go

@@ -235,3 +235,26 @@ func TestTransactionsFeaturesDetector_isIntrashard(t *testing.T) {
 		DestinationShard: 1,
 	}))
 }
+
+func TestTransactionsFeaturesDetector_isSmartContractResultIneffectiveRefund(t *testing.T) {
+	networkProvider := testscommon.NewNetworkProviderMock()
+	detector := newTransactionsFeaturesDetector(networkProvider)
+
+	require.True(t, detector.isSmartContractResultIneffectiveRefund(&transaction.ApiTransactionResult{
+		Sender:   testscommon.TestAddressOfContract,
+		Receiver: testscommon.TestAddressOfContract,
+		IsRefund: true,
+	}))
+
+	require.False(t, detector.isSmartContractResultIneffectiveRefund(&transaction.ApiTransactionResult{
+		Sender:   testscommon.TestAddressOfContract,
+		Receiver: testscommon.TestAddressOfContract,
+		IsRefund: false,
+	}))
+
+	require.False(t, detector.isSmartContractResultIneffectiveRefund(&transaction.ApiTransactionResult{
+		Sender:   testscommon.TestAddressOfContract,
+		Receiver: testscommon.TestAddressAlice,
+		IsRefund: false,
+	}))
+}

server/services/transactionsTransformer.go

@@ -114,6 +114,15 @@ func (transformer *transactionsTransformer) unsignedTxToRosettaTx(
 	scr *transaction.ApiTransactionResult,
 	txsInBlock []*transaction.ApiTransactionResult,
 ) *types.Transaction {
+	if transformer.featuresDetector.isSmartContractResultIneffectiveRefund(scr) {
+		log.Info("unsignedTxToRosettaTx: ineffective refund", "hash", scr.Hash)
+
+		return &types.Transaction{
+			TransactionIdentifier: hashToTransactionIdentifier(scr.Hash),
+			Operations:            []*types.Operation{},
+		}
+	}
+
 	if scr.IsRefund {
 		return &types.Transaction{
 			TransactionIdentifier: hashToTransactionIdentifier(scr.Hash),