Fixes unexecutable transaction#7635
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feat/supernova-async-exec #7635 +/- ##
=============================================================
- Coverage 77.66% 77.65% -0.01%
=============================================================
Files 877 877
Lines 121554 121612 +58
=============================================================
+ Hits 94404 94441 +37
- Misses 20909 20921 +12
- Partials 6241 6250 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return false, err | ||
| } | ||
|
|
||
| if atp.shardCoordinator.SelfId() == core.MetachainShardId { |
There was a problem hiding this comment.
move this after epoch check? before GetStorer
There was a problem hiding this comment.
This function also verifies that the transaction returned by the endpoint is included in a block that has an execution results.
If no execution result is found, the function returns an error, since the transaction may not have been executed yet.
So, If I move the check upper the check will be skipped.
| _, err := executionResultsStorer.GetFromEpoch(headerHash, miniblockMetadata.GetEpoch()) | ||
| return err | ||
| // check if the transaction miniblock metadata has a mb header on execution result | ||
| // if yes - the transaction was execution |
There was a problem hiding this comment.
| // if yes - the transaction was execution | |
| // if yes - the transaction was executed |
| } | ||
|
|
||
| for _, txHash := range unexecutableTxHashes { | ||
| cacheID := process.ShardCacherIdentifier(bp.shardCoordinator.SelfId(), bp.shardCoordinator.SelfId()) |
There was a problem hiding this comment.
will this work for cross shard? 🤔
There was a problem hiding this comment.
this will work also for the cross shard transactions, because the miniblock with the proposed transactions always is an intra shard miniblocks.
after execution we will have new miniblocks that can be cross shard.
There was a problem hiding this comment.
the miniblock yes, but the transactions I think are still in the proper pool, not all of them in self-self
There was a problem hiding this comment.
there is a common cache for all transactions with senders in own shard, and only those transactions can be un-executable.
| @@ -1827,17 +1842,17 @@ func (bp *baseProcessor) saveTxsToStorage(dataPool dataRetriever.ShardedDataCach | |||
| for _, txHash := range txHashes { | |||
There was a problem hiding this comment.
can we still at least log the error?
sstanculeanu
merged commit bfbc71b
into
feat/supernova-async-exec
3 participants
Reasoning behind the pull request
• Remove unexecutable transactions from the transaction cache once the block execution result is notarized
• Return the correct transaction status on the /transaction/:txHash endpoint for this case
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?