feat(input): utxo rpc input plugin#653
Open
cryptodj413 wants to merge 4 commits intoblinklabs-io:mainfrom
Open
feat(input): utxo rpc input plugin#653cryptodj413 wants to merge 4 commits intoblinklabs-io:mainfrom
cryptodj413 wants to merge 4 commits intoblinklabs-io:mainfrom
Conversation
Signed-off-by: cryptodj413 <shinjirohara2@gmail.com>
📝 WalkthroughWalkthroughAdds a new input plugin package input/utxorpc implementing a UTxO RPC gRPC client: plugin registration and cmdline options, Utxorpc type with New/Start/Stop and channel accessors, functional option helpers, FollowTip and WatchTx streaming with auto-reconnect/backoff, CBOR and protobuf mapping into adder events (blocks, transactions, governance, rollbacks), extensive protobuf-to-event helpers, unit tests, testdata, and a go.mod update adding connectrpc, utxorpc SDK, and a direct protobuf dependency. Possibly related issues
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="input/utxorpc/utxorpc.go">
<violation number="1" location="input/utxorpc/utxorpc.go:112">
P2: Guard the startup log call with a nil check to avoid a nil-pointer panic when the plugin is constructed without `WithLogger`.</violation>
<violation number="2" location="input/utxorpc/utxorpc.go:113">
P1: Do not log the raw API key; redact or omit it from structured logs.</violation>
<violation number="3" location="input/utxorpc/utxorpc.go:120">
P1: Track the run goroutine with `WaitGroup` before closing channels; otherwise Stop/restart can race with active sends.</violation>
</file>
<file name="input/utxorpc/plugin.go">
<violation number="1" location="input/utxorpc/plugin.go:129">
P2: `intersect-tip`/`intersect-point` are wired from CLI but never applied to FollowTip requests, so these new options are effectively no-ops.</violation>
</file>
<file name="input/utxorpc/protobuf.go">
<violation number="1" location="input/utxorpc/protobuf.go:109">
P2: Protobuf fallback governance proposals drop core fields (`rewardAccount`, `actionData`, `anchor`), producing incomplete governance events.</violation>
<violation number="2" location="input/utxorpc/protobuf.go:253">
P1: `BigUInt` conversion silently wraps on overflow via `Uint64()`, which can corrupt mapped amounts.</violation>
</file>
<file name="input/utxorpc/mapper.go">
<violation number="1" location="input/utxorpc/mapper.go:54">
P1: FollowTip Undo ignores `NativeBytes` blocks, so rollback events can be dropped when Undo is encoded in CBOR.</violation>
<violation number="2" location="input/utxorpc/mapper.go:198">
P1: WatchTx Undo only handles Cardano blocks and drops rollback events when Undo block context is provided as `NativeBytes`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (2)
input/utxorpc/plugin.go (1)
40-42: Consider passingNewFromCmdlineOptionsdirectly for consistency.Other input plugins (chainsync, mempool) pass
NewFromCmdlineOptionsdirectly as theNewFromOptionsFuncwithout an anonymous wrapper. While functionally equivalent, aligning with the existing pattern improves consistency.Suggested simplification
- NewFromOptionsFunc: func() plugin.Plugin { - return NewFromCmdlineOptions() - }, + NewFromOptionsFunc: NewFromCmdlineOptions,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@input/utxorpc/plugin.go` around lines 40 - 42, The NewFromOptionsFunc currently wraps NewFromCmdlineOptions in an anonymous function; change it to pass NewFromCmdlineOptions directly (i.e., set NewFromOptionsFunc = NewFromCmdlineOptions) to match the pattern used by other plugins and improve consistency—update the initialization where NewFromOptionsFunc and NewFromCmdlineOptions are referenced.input/utxorpc/options.go (1)
65-77: Consider documenting the interaction betweenWithIntersectTipandWithIntersectPoint.The documentation for
WithIntersectTipandWithIntersectPointcould clarify their precedence when both are set. Users may wonder which takes priority ifintersectTip=trueand anintersectPointis also provided.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@input/utxorpc/options.go` around lines 65 - 77, Update the docs for WithIntersectTip and WithIntersectPoint to state their interaction and precedence: in the comment for WithIntersectTip and/or WithIntersectPoint (functions named WithIntersectTip and WithIntersectPoint on type Utxorpc), explicitly document what happens when both are provided (e.g., that an explicit intersectPoint takes precedence over intersectTip, or vice versa), describe the behavior (which wins and whether the other is ignored or used as fallback), and mention any side-effects on stream negotiation so callers know which option to set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 28: The dependency line referencing the pre-release SDK
"github.com/utxorpc/go-sdk v0.0.2" must be explicitly accepted or changed;
either update the module to a stable release or add a clear, recorded acceptance
for using this v0.0.2 pre-release (e.g., a one-line note in the PR description
and a short justification in the repo like README or a contributors.md stating
you accept API instability for "github.com/utxorpc/go-sdk v0.0.2"), or
replace/pin it with a stable/ vendored commit if stability is required; make the
acceptance visible in the PR and commit history so reviewers know the
pre-release was approved.
In `@input/input.go`:
- Around line 18-22: The import block currently violates gci ordering; reorder
the three blank-imports so they satisfy linter grouping and alphabetical sorting
(e.g., sort the github.com/blinklabs-io/adder/input/* entries alphabetically:
chainsync, mempool, utxorpc) and ensure group separation/blank lines follow your
project's gci settings so the linter passes.
In `@input/utxorpc/mapper.go`:
- Around line 196-212: The undo handling currently only emits a rollback when
undo.GetBlock().GetCardano() is present, so rollbacks are lost if the server
supplies Block.NativeBytes; update the undo branch that checks resp.GetUndo() /
undo.GetBlock() to call watchTxBlockHeader() (the same helper used for apply) to
extract the header from either Block.Cardano or Block.NativeBytes and then
create the rollback event with event.NewRollbackEvent(common.NewPoint(...));
ensure you reuse watchTxBlockHeader() or its logic rather than duplicating
Cardano-only parsing so both native and cardano blocks produce the rollback
(also apply the same change to the later undo handling around lines 221-239).
- Around line 132-174: followTipApplyProtobuf currently emits reduced protobuf
payloads and misses per-certificate chainsync.drep.* fan-out and NativeBytes,
causing mismatch with followTipApplyCBOR; update the pb*Event builders (e.g.,
pbBlockEvent, pbTransactionEvent, pbGovernanceEvent) to populate the full
payloads including NativeBytes and any fields filled by the CBOR path, change
followTipApplyProtobuf to iterate certificates in each transaction and emit
chainsync.drep.* events the same way followTipApplyCBOR does (reusing the same
context/event shape), and broaden hasGovernanceData to detect governance data
from certificates as well as proposals so the protobuf fallback produces
event-for-event compatible output with the CBOR path.
- Around line 53-60: The code uses direct proto field access (b.Header) inside
the undo handling branch; replace all direct accesses with the proto getters
used elsewhere: change checks and uses of b.Header to b.GetHeader() and replace
b.Header.GetSlot()/b.Header.GetHash() with
b.GetHeader().GetSlot()/b.GetHeader().GetHash() in the resp.GetUndo() ->
undo.GetCardano() handling that constructs event.NewRollbackEvent so it complies
with the protogetter linter and proto best practices.
In `@input/utxorpc/protobuf.go`:
- Around line 162-184: The switch on protowire.Type in the function that scans
raw protobuf fields (the block handling wtype and txFeeFieldNumber) is missing
cases for protowire.StartGroupType and protowire.EndGroupType which the
exhaustive linter requires; add explicit cases for StartGroupType and
EndGroupType (e.g., mirror the default behavior by returning 0 or skipping
appropriately) so the switch is exhaustive and retains existing behavior for
unknown/deprecated group wire types.
In `@input/utxorpc/utxorpc.go`:
- Around line 223-225: The FollowTip request is built empty but should include
the stored intersection settings from the Utxorpc instance; populate the
syncpb.FollowTipRequest with u.intersectTip and/or u.intersectPoint before
calling u.client.FollowTipWithContext. In practice, update the creation of req
in the method that calls FollowTipWithContext so that the request's intersect
fields (e.g., IntersectTip/IntersectPoint or whichever fields the
syncpb.FollowTipRequest exposes) are set from the Utxorpc struct fields
(u.intersectTip / u.intersectPoint) so server restart/resume uses the configured
intersection rather than defaults.
- Around line 220-240: The loop blocks on stream.Receive() because the per-run
context cancel is not wired to Stop(), so Stop() cannot interrupt a blocked
Receive; create a per-run cancellable context (call it e.g. runCtx, runCancel)
when calling u.client.FollowTipWithContext (instead of a context only canceled
by defer), store runCancel in the receiver struct (or otherwise make it
reachable), and modify Stop() (and any pre-restart logic) to call runCancel
before waiting; also ensure you still defer runCancel after stream creation and
reuse the same pattern for the other FollowTip loop (the one that mirrors lines
262-281) so Receive() returns promptly when Stop() invokes the cancel.
- Around line 95-106: Stop() is mutating doneChan/eventChan/errorChan while the
run() goroutine isn't being tracked by u.wg, so the old goroutine can still
access channels being closed/replaced; fix by ensuring run() is registered with
the waitgroup when started (call u.wg.Add(1) where the goroutine is spawned and
defer u.wg.Done() at the top of run()), and in Stop() do lifecycle changes in
this order: signal stop (use u.stopOnce), wait for u.wg.Wait() to ensure run()
has exited, then close or recreate doneChan/eventChan/errorChan; also audit any
run*Once() helper invocations to ensure they happen inside the tracked goroutine
or after Wait() to avoid send-on-closed-channel races.
- Around line 112-113: The Start() startup log in Utxorpc is unguarded and leaks
the API key twice; update Start() to first nil-check u.logger (and return or
no-op if nil) before calling any logger methods, and change the log call to
avoid printing sensitive values: remove logging of u.apiKey and
headers[u.apiKeyHeader], instead log non-sensitive fields (e.g. u.url and
u.apiKeyHeader) and replace any API-key values with a redacted placeholder like
"<redacted>" or masked form; reference the Utxorpc receiver, Start() method,
u.logger, u.url, u.apiKeyHeader and headers[...] to locate and modify the code.
- Around line 290-293: Replace the inefficient fmt.Sprintf("%x", idle.GetHash())
call in the u.logger.Debug call with hex.EncodeToString(idle.GetHash())
(referencing u.logger.Debug and idle.GetHash) and add/import "encoding/hex" if
not already present; remove the fmt dependency if it becomes unused so the
logger uses hex.EncodeToString for the idle hash encoding.
---
Nitpick comments:
In `@input/utxorpc/options.go`:
- Around line 65-77: Update the docs for WithIntersectTip and WithIntersectPoint
to state their interaction and precedence: in the comment for WithIntersectTip
and/or WithIntersectPoint (functions named WithIntersectTip and
WithIntersectPoint on type Utxorpc), explicitly document what happens when both
are provided (e.g., that an explicit intersectPoint takes precedence over
intersectTip, or vice versa), describe the behavior (which wins and whether the
other is ignored or used as fallback), and mention any side-effects on stream
negotiation so callers know which option to set.
In `@input/utxorpc/plugin.go`:
- Around line 40-42: The NewFromOptionsFunc currently wraps
NewFromCmdlineOptions in an anonymous function; change it to pass
NewFromCmdlineOptions directly (i.e., set NewFromOptionsFunc =
NewFromCmdlineOptions) to match the pattern used by other plugins and improve
consistency—update the initialization where NewFromOptionsFunc and
NewFromCmdlineOptions are referenced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 490cf311-910a-4c33-beae-59c256d479e6
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
go.modinput/input.goinput/utxorpc/mapper.goinput/utxorpc/mapper_test.goinput/utxorpc/options.goinput/utxorpc/plugin.goinput/utxorpc/protobuf.goinput/utxorpc/testdata/shelley_block.hexinput/utxorpc/utxorpc.goinput/utxorpc/utxorpc_test.go
input/utxorpc/protobuf.go
Outdated
Comment on lines
+162
to
+184
| switch wtype { | ||
| case protowire.VarintType: | ||
| v, vn := protowire.ConsumeVarint(raw) | ||
| if vn < 0 { | ||
| return 0 | ||
| } | ||
| raw = raw[vn:] | ||
| if num == txFeeFieldNumber { | ||
| return v | ||
| } | ||
| case protowire.Fixed32Type: | ||
| raw = raw[4:] | ||
| case protowire.Fixed64Type: | ||
| raw = raw[8:] | ||
| case protowire.BytesType: | ||
| _, bn := protowire.ConsumeBytes(raw) | ||
| if bn < 0 { | ||
| return 0 | ||
| } | ||
| raw = raw[bn:] | ||
| default: | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
Add missing cases to satisfy exhaustive switch lint.
The pipeline fails because the switch on protowire.Type is missing protowire.StartGroupType and protowire.EndGroupType cases. While these are deprecated wire types rarely encountered, the exhaustive linter requires all enum cases to be handled explicitly.
Proposed fix
switch wtype {
case protowire.VarintType:
v, vn := protowire.ConsumeVarint(raw)
if vn < 0 {
return 0
}
raw = raw[vn:]
if num == txFeeFieldNumber {
return v
}
case protowire.Fixed32Type:
raw = raw[4:]
case protowire.Fixed64Type:
raw = raw[8:]
case protowire.BytesType:
_, bn := protowire.ConsumeBytes(raw)
if bn < 0 {
return 0
}
raw = raw[bn:]
+ case protowire.StartGroupType, protowire.EndGroupType:
+ // Deprecated group wire types; skip if encountered
+ return 0
default:
return 0
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| switch wtype { | |
| case protowire.VarintType: | |
| v, vn := protowire.ConsumeVarint(raw) | |
| if vn < 0 { | |
| return 0 | |
| } | |
| raw = raw[vn:] | |
| if num == txFeeFieldNumber { | |
| return v | |
| } | |
| case protowire.Fixed32Type: | |
| raw = raw[4:] | |
| case protowire.Fixed64Type: | |
| raw = raw[8:] | |
| case protowire.BytesType: | |
| _, bn := protowire.ConsumeBytes(raw) | |
| if bn < 0 { | |
| return 0 | |
| } | |
| raw = raw[bn:] | |
| default: | |
| return 0 | |
| } | |
| switch wtype { | |
| case protowire.VarintType: | |
| v, vn := protowire.ConsumeVarint(raw) | |
| if vn < 0 { | |
| return 0 | |
| } | |
| raw = raw[vn:] | |
| if num == txFeeFieldNumber { | |
| return v | |
| } | |
| case protowire.Fixed32Type: | |
| raw = raw[4:] | |
| case protowire.Fixed64Type: | |
| raw = raw[8:] | |
| case protowire.BytesType: | |
| _, bn := protowire.ConsumeBytes(raw) | |
| if bn < 0 { | |
| return 0 | |
| } | |
| raw = raw[bn:] | |
| case protowire.StartGroupType, protowire.EndGroupType: | |
| // Deprecated group wire types; skip if encountered | |
| return 0 | |
| default: | |
| return 0 | |
| } |
🧰 Tools
🪛 GitHub Actions: golangci-lint
[error] 162-162: golangci-lint: missing cases in switch of type protowire.Type: protowire.StartGroupType, protowire.EndGroupType (exhaustive)
🪛 GitHub Check: lint
[failure] 162-162:
missing cases in switch of type protowire.Type: protowire.StartGroupType, protowire.EndGroupType (exhaustive)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@input/utxorpc/protobuf.go` around lines 162 - 184, The switch on
protowire.Type in the function that scans raw protobuf fields (the block
handling wtype and txFeeFieldNumber) is missing cases for
protowire.StartGroupType and protowire.EndGroupType which the exhaustive linter
requires; add explicit cases for StartGroupType and EndGroupType (e.g., mirror
the default behavior by returning 0 or skipping appropriately) so the switch is
exhaustive and retains existing behavior for unknown/deprecated group wire
types.
Signed-off-by: cryptodj413 <shinjirohara2@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
input/utxorpc/mapper.go (1)
158-179:⚠️ Potential issue | 🟠 MajorProtobuf paths still drop per-certificate DRep events.
followTipApplyCBOR()fans outinput.drep-*, but these branches stop atinput.transaction/input.governance. That meanswatch-txcan never emit the DRep events promised by the plugin, andfollow-tiploses them whenever the provider omitsNativeBytes. Reuse the extracted protobuf certificate data here and fan it out withevent.DRepEventType()the same way the CBOR path does.Also applies to: 274-289
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@input/utxorpc/mapper.go` around lines 158 - 179, The protobuf path in the tx loop (inside followTipApplyCBOR / mapper.go where you iterate body.GetTx()) emits only "input.transaction" and "input.governance" and thus drops per-certificate DRep events; fix by reusing the extracted protobuf certificate data for each tx (the same data used when CBOR NativeBytes is present) and fan out additional events using event.DRepEventType() after creating the transaction/governance events. Concretely, in the loop that calls pbTransactionContext(header, txHash, idx, networkMagic) / pbTransactionEvent(...) and pbGovernanceContext(...) / pbGovernanceEvent(...), detect and extract the certificate/DRep protobuf payload (as the CBOR branch does) and append corresponding event.New("input.drep-...", now, <appropriate pbContext>, <drep pb event>) using event.DRepEventType() so the protobuf-only path mirrors the CBOR path; reuse existing helpers (pbTransactionContext, pbGovernanceContext, hasGovernanceData) to locate where to insert the DRep fan-out for both the shown loop and the similar block at the other location (lines 274-289).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@input/utxorpc/protobuf.go`:
- Around line 218-220: Validate NewCommitteeThreshold's numerator/denominator
for negativity before casting to uint64: when handling
uca.GetNewCommitteeThreshold() in protobuf.go (the block that assigns
d.QuorumNumerator and d.QuorumDenominator), check qt.GetNumerator() and
qt.GetDenominator() are >= 0 and reject or return an error (or fallback to a
safe default) if either is negative, instead of directly casting to uint64; this
prevents negative values from wrapping into huge unsigned numbers.
- Around line 381-415: The deposit uint64->int64 casts can wrap negative when
utxorpcBigIntToUint64 returns > math.MaxInt64; replace direct
int64(utxorpcBigIntToUint64(...)) uses with a clamping helper (e.g.
clampUint64ToInt64(v uint64) int64 that returns math.MaxInt64 for
v>math.MaxInt64 else int64(v)). Update the Deposit assignments in the
RegDrepCert and UnregDrepCert DRepCertificateData construction and the coin
arguments passed into pbVoteDelegation in VoteRegDelegCert and
StakeVoteRegDelegCert to use clampUint64ToInt64(utxorpcBigIntToUint64(...)).
Ensure clampUint64ToInt64 is defined and used consistently where deposits are
converted.
---
Duplicate comments:
In `@input/utxorpc/mapper.go`:
- Around line 158-179: The protobuf path in the tx loop (inside
followTipApplyCBOR / mapper.go where you iterate body.GetTx()) emits only
"input.transaction" and "input.governance" and thus drops per-certificate DRep
events; fix by reusing the extracted protobuf certificate data for each tx (the
same data used when CBOR NativeBytes is present) and fan out additional events
using event.DRepEventType() after creating the transaction/governance events.
Concretely, in the loop that calls pbTransactionContext(header, txHash, idx,
networkMagic) / pbTransactionEvent(...) and pbGovernanceContext(...) /
pbGovernanceEvent(...), detect and extract the certificate/DRep protobuf payload
(as the CBOR branch does) and append corresponding event.New("input.drep-...",
now, <appropriate pbContext>, <drep pb event>) using event.DRepEventType() so
the protobuf-only path mirrors the CBOR path; reuse existing helpers
(pbTransactionContext, pbGovernanceContext, hasGovernanceData) to locate where
to insert the DRep fan-out for both the shown loop and the similar block at the
other location (lines 274-289).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d2aabe4-c4dd-4e71-9d57-f6fbea6a3539
📒 Files selected for processing (5)
input/utxorpc/mapper.goinput/utxorpc/mapper_test.goinput/utxorpc/protobuf.goinput/utxorpc/utxorpc.goinput/utxorpc/utxorpc_test.go
Signed-off-by: cryptodj413 <shinjirohara2@gmail.com>
Signed-off-by: cryptodj413 <shinjirohara2@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
input/utxorpc/mapper.go (1)
269-299: Consider documenting empty header fallback behavior.When
headeris nil (line 274-276), an emptyBlockHeader{}is used, resulting in events with zero slot/hash values. This is valid but may confuse downstream consumers expecting populated block context. The current behavior is acceptable for watch-tx mode where block context may genuinely be unavailable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@input/utxorpc/mapper.go` around lines 269 - 299, The function watchTxApplyProtobuf currently substitutes a nil header with an empty cardanopb.BlockHeader, producing events whose block context (slot/hash) is zero; update watchTxApplyProtobuf to document this fallback behavior by adding a clear comment above the nil-check that explains why header is set to &cardanopb.BlockHeader{}, that this yields zeroed block context in pbTransactionContext and pbGovernanceContext, and that this is intentional for watch-tx mode where block info may be unavailable so downstream consumers should handle zero/empty block fields accordingly.input/utxorpc/protobuf.go (1)
496-511: Consider handling negativeBigInt.Intvalues explicitly.When
b.GetInt()returns a negative value, it falls through toGetBigUInt()and may return 0 if that's also empty. For amounts (fees, deposits), this silent conversion to 0 may mask malformed data. Consider logging or returning an error for negative values if they shouldn't occur.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@input/utxorpc/protobuf.go` around lines 496 - 511, utxorpcBigIntToUint64 currently treats negative b.GetInt() as if it were absent and may silently return 0; update the function to detect when v := b.GetInt() is negative and handle it explicitly (e.g., log the malformed negative value and return a sentinel such as math.MaxUint64 or another clearly-invalid uint64) instead of falling through to b.GetBigUInt(); modify the branch around b.GetInt() to check v < 0 and perform the chosen explicit handling, referencing utxorpcBigIntToUint64, b.GetInt(), b.GetBigUInt(), and math.MaxUint64 so reviewers can locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@input/utxorpc/utxorpc.go`:
- Around line 105-116: The Start() restart sequence is not protected from
concurrent Stop() calls (risking double-close or race on u.doneChan, u.stopOnce
and u.wg); add a lifecycle mutex (e.g. lifecycleMu sync.Mutex) to the struct and
acquire it at the start of both Start() and Stop() to serialize lifecycle
transitions, then perform reset of u.stopOnce, close/create u.doneChan, create
eventChan/errorChan and wg.Wait() while holding the lock (or hold for the
minimal critical section and document any unlocked waits), ensuring you only
close channels when non-nil and not already closed; update Start()/Stop() to use
the mutex to prevent races between those symbols (u.stopOnce, u.doneChan, u.wg,
u.eventChan, u.errorChan).
---
Nitpick comments:
In `@input/utxorpc/mapper.go`:
- Around line 269-299: The function watchTxApplyProtobuf currently substitutes a
nil header with an empty cardanopb.BlockHeader, producing events whose block
context (slot/hash) is zero; update watchTxApplyProtobuf to document this
fallback behavior by adding a clear comment above the nil-check that explains
why header is set to &cardanopb.BlockHeader{}, that this yields zeroed block
context in pbTransactionContext and pbGovernanceContext, and that this is
intentional for watch-tx mode where block info may be unavailable so downstream
consumers should handle zero/empty block fields accordingly.
In `@input/utxorpc/protobuf.go`:
- Around line 496-511: utxorpcBigIntToUint64 currently treats negative
b.GetInt() as if it were absent and may silently return 0; update the function
to detect when v := b.GetInt() is negative and handle it explicitly (e.g., log
the malformed negative value and return a sentinel such as math.MaxUint64 or
another clearly-invalid uint64) instead of falling through to b.GetBigUInt();
modify the branch around b.GetInt() to check v < 0 and perform the chosen
explicit handling, referencing utxorpcBigIntToUint64, b.GetInt(),
b.GetBigUInt(), and math.MaxUint64 so reviewers can locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1b116bba-06b8-4b0f-a590-4582429a0f77
📒 Files selected for processing (3)
input/utxorpc/mapper.goinput/utxorpc/protobuf.goinput/utxorpc/utxorpc.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #632
Summary by cubic
Adds a new
utxorpcinput plugin to stream Cardano blocks and transactions over gRPC from a UTxO RPC provider. Supportsfollow-tipandwatch-txwith rollback, auto-reconnect, optional CBOR, and network-aware context.New Features
input/utxorpcplugin with options:url,mode(follow-tip|watch-tx),network,api-key-header,api-key,intersect-tip,intersect-point(supports multiple points),auto-reconnect,include-cbor.input.block,input.transaction,input.governance, and DRep certificate events (input.drep-registration,input.drep-update,input.drep-retirement); handles Undo/Reset asinput.rollback.input/input.go; includes unit tests and Shelley block test data.Dependencies
connectrpc.com/connectandgithub.com/utxorpc/go-sdk.google.golang.org/protobufto a direct dependency; updatego.mod/go.sum.Written for commit 33d822b. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests