chore: remove candidate transactions and deprecated preconfirmed api#3732
chore: remove candidate transactions and deprecated preconfirmed api#3732EgeCaner wants to merge 1 commit into
Conversation
|
Claude finished @EgeCaner's task in 4m 11s —— View job PR Review: remove candidate transactions and deprecated preconfirmed API
This is a clean, well-scoped removal. The Go packages all build ( Important
Nit / question
Nothing here blocks the PR. The candidate removal looks correct and the test reduction tracks the deleted code paths. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| stateDiff, err := AdaptStateDiff(response.TransactionStateDiffs[i]) | ||
| if err != nil { | ||
| return pending.PreConfirmed{}, err | ||
| } | ||
| txStateDiffs[preIdx] = &stateDiff | ||
| receipts[preIdx] = AdaptTransactionReceipt(response.Receipts[i]) | ||
| txStateDiffs[i] = &stateDiff | ||
| receipts[i] = AdaptTransactionReceipt(response.Receipts[i]) | ||
| eventCount += uint64(len(response.Receipts[i].Events)) |
There was a problem hiding this comment.
The length check above (L517-523) guarantees the three slices have equal length, but it does not guarantee that individual elements are non-nil. With candidate handling removed, this loop now unconditionally dereferences each element:
AdaptStateDiff(response.TransactionStateDiffs[i])panics if the entry isnil(AdaptStateDiffreadsresponse.OldDeclaredContractswith no nil guard).response.Receipts[i].Events(L544) is a nil-pointer deref if the receipt isnil.
A nil entry was precisely the old "candidate tx" signal (IsCandidateTx). This is safe only if the new PreConfirmedBlockWithIdentifier endpoint never returns partial/candidate slots — which is the documented API contract. Since this input comes from the (external) feeder gateway, a malformed/unexpected response would now panic the adapter instead of erroring out.
Consider guarding explicitly so a bad payload returns an error rather than crashing, e.g.:
if response.TransactionStateDiffs[i] == nil || response.Receipts[i] == nil {
return pending.PreConfirmed{}, errors.New("pre_confirmed block contains a tx with missing receipt or state diff")
}Note: the previous explicit contract assertion in TestAdaptPreConfirmedWithDelta (the require.NotNilf loop over receipts) was also removed in this PR, so neither the code nor the test now defends against this case.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3732 +/- ##
==========================================
- Coverage 75.13% 75.11% -0.02%
==========================================
Files 425 424 -1
Lines 38331 38221 -110
==========================================
- Hits 28799 28709 -90
+ Misses 7538 7525 -13
+ Partials 1994 1987 -7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
EgeCaner
force-pushed
the
chore/remove-candidate-txs-and-deprecated-pconf-api
branch
from
8bd2e59 to
dc99137
Compare
|
Claude finished @EgeCaner's task in 2m 50s —— View job PR Review: remove candidate transactions and deprecated preconfirmed API
This is a clean, well-scoped removal. Affected Go packages build ( Important
Nit
Nothing here blocks the PR. The candidate removal looks correct and the test reduction tracks the deleted code paths. |
| stateDiff, err := AdaptStateDiff(response.TransactionStateDiffs[i]) | ||
| if err != nil { | ||
| return pending.PreConfirmed{}, err | ||
| } | ||
| txStateDiffs[preIdx] = &stateDiff | ||
| receipts[preIdx] = AdaptTransactionReceipt(response.Receipts[i]) | ||
| txStateDiffs[i] = &stateDiff | ||
| receipts[i] = AdaptTransactionReceipt(response.Receipts[i]) | ||
| eventCount += uint64(len(response.Receipts[i].Events)) |
There was a problem hiding this comment.
With candidate handling removed, this loop now unconditionally dereferences each per-tx slot. The size check at L517-523 only guarantees the three slices have equal length — not that individual elements are non-nil. A nil entry (previously the candidate signal via IsCandidateTx) would now panic rather than error:
AdaptStateDiff(response.TransactionStateDiffs[i])panics —AdaptStateDiffreadsresponse.OldDeclaredContracts(L457) with no nil guard.response.Receipts[i].Events(L544) is a nil-pointer deref if the receipt isnil(AdaptTransactionReceiptitself handles nil, but this line doesn't).
Since this payload comes from the external feeder gateway, a malformed/unexpected response would crash the adapter instead of returning an error. The previous require.NotNilf contract assertion in TestAdaptPreConfirmedWithDelta was also dropped, so neither code nor test now defends this. Consider an explicit guard:
| stateDiff, err := AdaptStateDiff(response.TransactionStateDiffs[i]) | |
| if err != nil { | |
| return pending.PreConfirmed{}, err | |
| } | |
| txStateDiffs[preIdx] = &stateDiff | |
| receipts[preIdx] = AdaptTransactionReceipt(response.Receipts[i]) | |
| txStateDiffs[i] = &stateDiff | |
| receipts[i] = AdaptTransactionReceipt(response.Receipts[i]) | |
| eventCount += uint64(len(response.Receipts[i].Events)) | |
| if response.TransactionStateDiffs[i] == nil || response.Receipts[i] == nil { | |
| return pending.PreConfirmed{}, errors.New("pre_confirmed block contains a tx with missing receipt or state diff") | |
| } | |
| stateDiff, err := AdaptStateDiff(response.TransactionStateDiffs[i]) | |
| if err != nil { | |
| return pending.PreConfirmed{}, err | |
| } | |
| txStateDiffs[i] = &stateDiff | |
| receipts[i] = AdaptTransactionReceipt(response.Receipts[i]) | |
| eventCount += uint64(len(response.Receipts[i].Events)) |
1 participant
No description provided.