feat(mempool): resolve inputs from mempool for chained txs#637
Merged
cryptodj413 merged 4 commits intoblinklabs-io:mainfrom Mar 1, 2026
Merged
feat(mempool): resolve inputs from mempool for chained txs#637cryptodj413 merged 4 commits intoblinklabs-io:mainfrom
cryptodj413 merged 4 commits intoblinklabs-io:mainfrom
Conversation
Signed-off-by: cryptodj413 <shinjirohara2@gmail.com>
📝 WalkthroughWalkthroughThe mempool module adds an unexported Possibly related PRs
🚥 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}
Contributor
Author
|
Closes #627 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/mempool/mempool.go`:
- Around line 340-357: The code discards partially-resolved inputs when
resolveTransactionInputs fails on a Kupo fetch; change resolveTransactionInputs
to accumulate and return any successfully resolved inputs even if some Kupo
lookups error (log and skip failing inputs rather than returning nil), and
update the caller loop in mempool.go (the section building ctx/payload and
calling resolveTransactionInputs) to set payload.ResolvedInputs when the
returned slice has entries regardless of a non-fatal error, while still logging
the error; specifically modify resolveTransactionInputs to continue on per-input
Kupo errors (or return (resolvedSlice, err) instead of nil) and adjust the
caller to use resolved inputs when len(resolvedInputs)>0 even if err != nil.
Signed-off-by: cryptodj413 <shinjirohara2@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
input/mempool/mempool.go (1)
442-502:⚠️ Potential issue | 🟠 MajorPipeline failure: error return is always
nil— either drop it or use it.The linter (
unparam) flags that the second return value is alwaysnil, which also makes the caller'sif err != nilbranch (lines 351-354) dead code. Since the current design intentionally continues past every failure to preserve partial results, the cleanest fix is to remove the error return.Option 1 (preferred): drop the error return
In
resolveTransactionInputs:-func (m *Mempool) resolveTransactionInputs(tx ledger.Transaction, mempoolUtxo map[string]ledger.TransactionOutput) ([]ledger.TransactionOutput, error) { +func (m *Mempool) resolveTransactionInputs(tx ledger.Transaction, mempoolUtxo map[string]ledger.TransactionOutput) []ledger.TransactionOutput {- return resolvedInputs, nil + return resolvedInputsAnd in the caller (
pollOnce):- resolvedInputs, err := m.resolveTransactionInputs(p.tx, mempoolUtxo) - if err != nil { - if m.logger != nil { - m.logger.Warn("failed to resolve transaction inputs, emitting without resolved inputs", "error", err) - } - } else if len(resolvedInputs) > 0 { + resolvedInputs := m.resolveTransactionInputs(p.tx, mempoolUtxo) + if len(resolvedInputs) > 0 { payload.ResolvedInputs = resolvedInputs }Option 2: accumulate and return a joined error for observability
If you want the caller to know that some inputs couldn't be resolved (e.g. for metrics or future use), accumulate errors and return them:
+ var errs []error // ... inside the loop, where you currently `continue` on Kupo failures: - continue + errs = append(errs, fmt.Errorf("input %s:%d: %w", txID, txIndex, err)) + continue // ... - return resolvedInputs, nil + return resolvedInputs, errors.Join(errs...)The caller would then handle a non-nil error as a partial-failure warning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@input/mempool/mempool.go` around lines 442 - 502, The function resolveTransactionInputs currently returns ([]ledger.TransactionOutput, error) but always returns nil error; change its signature to return only []ledger.TransactionOutput and remove all error handling/returns inside it (keep the logic that continues on Kupo/client failures), then update its callers (notably pollOnce which checks "if err != nil" around the resolveTransactionInputs call) to accept the single return value and drop the dead err handling branch; ensure you update all references to resolveTransactionInputs and rebuild to fix compile errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@input/mempool/mempool.go`:
- Around line 442-502: The function resolveTransactionInputs currently returns
([]ledger.TransactionOutput, error) but always returns nil error; change its
signature to return only []ledger.TransactionOutput and remove all error
handling/returns inside it (keep the logic that continues on Kupo/client
failures), then update its callers (notably pollOnce which checks "if err !=
nil" around the resolveTransactionInputs call) to accept the single return value
and drop the dead err handling branch; ensure you update all references to
resolveTransactionInputs and rebuild to fix compile errors.
Signed-off-by: cryptodj413 <shinjirohara2@gmail.com>
cryptodj413
force-pushed
the
feat/mempool-support
branch
from
cc744d4 to
c09f89e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
input/mempool/mempool.go (1)
441-503: Well-structured mempool-first resolution with graceful Kupo fallback.The refactored method correctly prioritizes the in-poll mempool UTXO map before falling back to Kupo, and the error-accumulation pattern (lines 447, 465-466, 484-485, 490-491, 499-501) ensures partial results are preserved. This is a solid improvement over the previous hard-return-on-error behavior.
One minor observation: when Kupo is not configured and an input isn't found in the mempool UTXO (lines 460-462), the input is silently skipped. This is fine for the "mempool-only" mode since there's nothing else to try, but there's no diagnostic logging for this case. Consider a
Debug-level log so operators can distinguish "input not in mempool, no Kupo" from "input resolved from mempool."💡 Optional: add debug log for unresolvable inputs in mempool-only mode
// Fall back to Kupo for on-chain outputs. if m.kupoUrl == "" || m.kupoDisabled { + if m.logger != nil { + m.logger.Debug("input not in mempool UTXO and Kupo unavailable, skipping", "txId", txID, "index", txIndex) + } continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@input/mempool/mempool.go` around lines 441 - 503, The method resolveTransactionInputs silently skips inputs when Kupo is unavailable (m.kupoUrl == "" || m.kupoDisabled) and the input isn't in mempoolUtxo; add a Debug log via m.logger (if non-nil) inside that branch to record the skipped input (include txID, txIndex and key or pattern) so operators can distinguish "not in mempool" vs resolved cases; update only the branch in resolveTransactionInputs that currently does "continue" when Kupo is disabled to emit this debug message.
🤖 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/mempool/mempool.go`:
- Around line 114-118: The two logger.Info calls that reference m.kupoUrl are
missing a nil check on m.logger and can cause a nil-pointer panic because New()
doesn't set a default logger; update the code around the m.kupoUrl check to
guard both branches with if m.logger != nil before calling m.logger.Info (i.e.,
ensure any logging that uses m.logger in this block checks m.logger != nil),
keeping the same messages for the empty and non-empty m.kupoUrl branches so
behavior is unchanged when a logger is present.
---
Nitpick comments:
In `@input/mempool/mempool.go`:
- Around line 441-503: The method resolveTransactionInputs silently skips inputs
when Kupo is unavailable (m.kupoUrl == "" || m.kupoDisabled) and the input isn't
in mempoolUtxo; add a Debug log via m.logger (if non-nil) inside that branch to
record the skipped input (include txID, txIndex and key or pattern) so operators
can distinguish "not in mempool" vs resolved cases; update only the branch in
resolveTransactionInputs that currently does "continue" when Kupo is disabled to
emit this debug message.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/mempool/mempool.go`:
- Around line 111-125: Reset the Kupo-related state at the top of Start(): clear
m.kupoDisabled, m.kupoInvalidPatternLogged and set m.kupoClient = nil (and
re-evaluate m.kupoUrl if needed) so a prior Stop() or previous run that disabled
Kupo doesn't persist across restarts; update the Start() method to explicitly
reinitialize these fields before any logic that checks or initializes the kupo
client.
Signed-off-by: cryptodj413 <shinjirohara2@gmail.com>
cryptodj413
force-pushed
the
feat/mempool-support
branch
from
6ec4309 to
45d4fe0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
input/mempool/mempool.go (1)
447-447: Extract a shared UTxO-key builder to prevent format drift.The same key format is duplicated in two locations; centralizing it makes future changes safer.
♻️ Suggested refactor
+func mempoolUtxoKey(txID string, index int) string { + return txID + ":" + strconv.Itoa(index) +} + func (m *Mempool) buildMempoolUtxo(pollTxs []pollTx) map[string]ledger.TransactionOutput { utxo := make(map[string]ledger.TransactionOutput) for _, p := range pollTxs { txID := p.hash for idx, out := range p.tx.Outputs() { - key := txID + ":" + strconv.Itoa(idx) + key := mempoolUtxoKey(txID, idx) utxo[key] = out } } return utxo } @@ - key := txID + ":" + strconv.Itoa(txIndex) + key := mempoolUtxoKey(txID, txIndex)Also applies to: 464-464
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@input/mempool/mempool.go` at line 447, Extract the UTxO key construction into a single helper function (e.g., buildUTxOKey or makeUTxOKey) and replace the inline expressions like txID + ":" + strconv.Itoa(idx) in mempool.go (occurrences around the current key assignment and the other duplicate at the second location) with calls to that helper; implement the helper to consistently format the key (for example using fmt.Sprintf("%s:%d", txID, idx)) and update any imports if needed so all places use the single function to prevent format drift.
🤖 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/mempool/mempool.go`:
- Around line 476-480: The code calls m.getKupoClient() inside the per-input
loop, causing repeated client initialization/health checks when Kupo is down;
move the call out of the loop so you obtain the Kupo client once per transaction
resolution (e.g., call k, err := m.getKupoClient() before iterating inputs),
handle a single error case by appending to resolveErrs and skipping the whole
input-processing loop for that tx, and then reuse the same k variable inside the
loop where resolveErrs currently appends errors (references: m.getKupoClient(),
resolveErrs, txID, txIndex).
---
Nitpick comments:
In `@input/mempool/mempool.go`:
- Line 447: Extract the UTxO key construction into a single helper function
(e.g., buildUTxOKey or makeUTxOKey) and replace the inline expressions like txID
+ ":" + strconv.Itoa(idx) in mempool.go (occurrences around the current key
assignment and the other duplicate at the second location) with calls to that
helper; implement the helper to consistently format the key (for example using
fmt.Sprintf("%s:%d", txID, idx)) and update any imports if needed so all places
use the single function to prevent format drift.
Comment on lines
+476
to
+480
| k, err := m.getKupoClient() | ||
| if err != nil { | ||
| resolveErrs = append(resolveErrs, fmt.Errorf("input %s:%d kupo client: %w", txID, txIndex, err)) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Avoid per-input Kupo client initialization retries.
m.getKupoClient() is called inside the input loop. When Kupo is down, each unresolved input repeats client init/health-check work, amplifying latency and error volume for a single tx.
🔧 Suggested fix (fetch Kupo client once per transaction resolution)
func (m *Mempool) resolveTransactionInputs(tx ledger.Transaction, mempoolUtxo map[string]ledger.TransactionOutput) ([]ledger.TransactionOutput, error) {
var resolvedInputs []ledger.TransactionOutput
var resolveErrs []error
+ var kupoClient *kugo.Client
+ var kupoClientErr error
+ kupoChecked := false
for _, input := range tx.Inputs() {
txID := input.Id().String()
txIndex := int(input.Index())
key := txID + ":" + strconv.Itoa(txIndex)
// Resolve from mempool first (chained txs: both in same poll).
if out, ok := mempoolUtxo[key]; ok {
resolvedInputs = append(resolvedInputs, out)
continue
}
// Fall back to Kupo for on-chain outputs.
if m.kupoUrl == "" || m.kupoDisabled {
continue
}
- k, err := m.getKupoClient()
- if err != nil {
- resolveErrs = append(resolveErrs, fmt.Errorf("input %s:%d kupo client: %w", txID, txIndex, err))
- continue
- }
+ if !kupoChecked {
+ kupoChecked = true
+ kupoClient, kupoClientErr = m.getKupoClient()
+ if kupoClientErr != nil {
+ resolveErrs = append(resolveErrs, fmt.Errorf("kupo client: %w", kupoClientErr))
+ }
+ }
+ if kupoClientErr != nil {
+ continue
+ }
pattern := fmt.Sprintf("%d@%s", txIndex, txID)
ctx, cancel := context.WithTimeout(context.Background(), defaultKupoTimeout)
- matches, err := k.Matches(ctx, kugo.Pattern(pattern))
+ matches, err := kupoClient.Matches(ctx, kugo.Pattern(pattern))
cancel()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@input/mempool/mempool.go` around lines 476 - 480, The code calls
m.getKupoClient() inside the per-input loop, causing repeated client
initialization/health checks when Kupo is down; move the call out of the loop so
you obtain the Kupo client once per transaction resolution (e.g., call k, err :=
m.getKupoClient() before iterating inputs), handle a single error case by
appending to resolveErrs and skipping the whole input-processing loop for that
tx, and then reuse the same k variable inside the loop where resolveErrs
currently appends errors (references: m.getKupoClient(), resolveErrs, txID,
txIndex).
wolf31o2
approved these changes
Feb 28, 2026
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
2 participants
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.
Closes #627
Summary by cubic
Resolves inputs for chained mempool transactions from a per-poll UTxO, falling back to Kupo only for on-chain outputs. Resets Kupo state on start and emits transactions with any inputs that can be resolved, with clear partial-resolution logging.
Written for commit 45d4fe0. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Refactor