Skip to content

Comments

Add thorchain support#382

Open
neavra wants to merge 8 commits intomainfrom
362-add-thorchain-support
Open

Add thorchain support#382
neavra wants to merge 8 commits intomainfrom
362-add-thorchain-support

Conversation

@neavra
Copy link
Contributor

@neavra neavra commented Nov 12, 2025

Summary by CodeRabbit

  • New Features

    • Added THORChain blockchain support
    • Enabled THORChain transaction validation and on-chain status checking
    • Added configurable THORChain RPC endpoint
  • Chores

    • Updated module dependencies
    • Introduced golang.org/x/time v0.9.0 as a direct dependency

@neavra neavra linked an issue Nov 12, 2025 that may be closed by this pull request
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 12, 2025

Walkthrough

Adds THORChain support: new RPC client, indexer, config entry, key-sign validation handling, transaction hash computation, and retry-capable on-chain status checks; also updates module dependencies in go.mod.

Changes

Cohort / File(s) Summary
Dependency
go.mod
Bumped github.com/vultisig/recipes version and added golang.org/x/time v0.9.0 as a direct requirement.
Validation & API
internal/api/plugin.go
Added THORChain case to key-sign validation switch; decodes base64 proposed THORChain tx into txBytesEvaluate.
Config
plugin/tx_indexer/pkg/config/config.go
Added THORChain RpcItem field to RpcConfig (mapstructure:"thorchain" json:"thorchain,omitempty").
Chain wiring
plugin/tx_indexer/chains_list.go
Conditionally creates a THORChain RPC client when configured and registers a THORChain indexer in the chains list.
Indexer implementation
plugin/tx_indexer/pkg/chain/thorchain.go
New THORChainIndexer type with NewTHORChainIndexer(sdk *thorchain.SDK) and ComputeTxHash(proposedTx []byte, sigs map[string]tss.KeysignResponse) (string, error) that signs via SDK and returns uppercase SHA-256 hex hash.
RPC client
plugin/tx_indexer/pkg/rpc/thorchain.go
New THORChain HTTP client with rate limiter; NewTHORChain(rpcURL string) and GetTxStatus(ctx, txHash string) implementing retries, exponential backoff with jitter, Retry-After handling, HTTP error handling, and mapping responses to TxOnChainStatus.
Examples
tx_indexer.example.json
Added thorchain RPC endpoint entry under rpc section.

Sequence Diagram

sequenceDiagram
    participant API as Key-Sign API
    participant Validator as Plugin Validator
    participant Indexer as THORChain Indexer
    participant SDK as THOR SDK
    participant RPC as THORChain RPC

    API->>Validator: Submit THORChain key-sign request
    Validator->>Validator: base64 decode proposed tx -> txBytesEvaluate
    Validator-->>API: validation result / error

    Note over Indexer,SDK: ComputeTxHash flow
    Indexer->>SDK: Sign proposedTx with sigs
    alt sign success
        SDK-->>Indexer: signedTx
        Indexer->>Indexer: SHA-256(signedTx) -> uppercase hex
        Indexer-->>API: tx hash
    else sign error
        SDK-->>Indexer: error
        Indexer-->>API: error
    end

    Note over API,RPC: On-chain status query
    API->>RPC: GetTxStatus(txHash)
    rect rgba(220,240,255,0.4)
        RPC->>RPC: enforce rate limiter
        RPC->>RPC: makeRequestWithRetry (exp backoff + jitter)
        RPC->>RPC: handle 429 Retry-After, non-200, parse tx_result
    end
    RPC-->>API: TxOnChainStatus (Pending/Success/Fail)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Files needing extra attention:
    • plugin/tx_indexer/pkg/rpc/thorchain.go: verify retry/backoff correctness, Retry-After parsing, proper handling of 429 and other HTTP statuses, and context cancellation during backoffs.
    • plugin/tx_indexer/pkg/chain/thorchain.go: confirm signing invocation, error wrapping, and SHA-256 hash derivation match THORChain/CometBFT expectations.
    • plugin/tx_indexer/chains_list.go: ensure conditional RPC creation and indexer registration align with existing initialization and error handling patterns.
    • internal/api/plugin.go: check base64 decoding error message and consistency with other chain cases.

Poem

🐇 I hopped through code to add Thor's trail,
I signed each bite and hashed the tale,
Retries hummed softly, limits kept tight,
Now Thor's txs wake and take to flight. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add thorchain support' accurately and concisely summarizes the main objective of the pull request, which is to introduce THORChain support across the codebase.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 362-add-thorchain-support

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b811fd7 and d7c4674.

📒 Files selected for processing (1)
  • internal/api/plugin.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/api/plugin.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@neavra neavra marked this pull request as ready for review November 13, 2025 09:25
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
plugin/tx_indexer/pkg/rpc/thorchain.go (1)

146-148: Fix jitter calculation so it actually introduces variance

Line 147 casts the random factor to time.Duration before multiplying, so it truncates to 0 ns and the backoff never jitters. Use float arithmetic first, then convert back to time.Duration.

-	jitter := time.Duration(rand.Float64()*0.5-0.25) * delay
-	return delay + jitter
+	scale := 1 + (rand.Float64()*0.5 - 0.25)
+	return time.Duration(float64(delay) * scale)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2178b62 and 272d7a6.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (7)
  • go.mod (1 hunks)
  • internal/api/plugin.go (1 hunks)
  • plugin/tx_indexer/chains_list.go (3 hunks)
  • plugin/tx_indexer/pkg/chain/thorchain.go (1 hunks)
  • plugin/tx_indexer/pkg/config/config.go (1 hunks)
  • plugin/tx_indexer/pkg/rpc/thorchain.go (1 hunks)
  • tx_indexer.example.json (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-06-10T19:00:10.934Z
Learnt from: webpiratt
Repo: vultisig/verifier PR: 156
File: tx_indexer/chains_list.go:22-37
Timestamp: 2025-06-10T19:00:10.934Z
Learning: The tx_indexer component must have every supported RPC (currently Bitcoin and Ethereum) configured; partial configuration is considered invalid and the application should fail fast if any RPC initialisation fails.

Applied to files:

  • plugin/tx_indexer/chains_list.go
  • plugin/tx_indexer/pkg/chain/thorchain.go
  • tx_indexer.example.json
📚 Learning: 2025-08-17T04:27:23.578Z
Learnt from: neavra
Repo: vultisig/verifier PR: 316
File: internal/api/plugin.go:26-26
Timestamp: 2025-08-17T04:27:23.578Z
Learning: In the vultisig/verifier project, the migration to vultisig-go/common was selective - only certain utilities (GetQueryParam, GetUUIDParam, IsValidSortField, address derivation, crypto functions) were moved to the shared library. Other functionality like vultisig_validator and SystemMigrations intentionally remained in the original github.com/vultisig/verifier/common package and should not be flagged as stale imports.

Applied to files:

  • plugin/tx_indexer/chains_list.go
  • go.mod
📚 Learning: 2025-06-10T18:57:04.359Z
Learnt from: webpiratt
Repo: vultisig/verifier PR: 156
File: internal/api/plugin.go:72-86
Timestamp: 2025-06-10T18:57:04.359Z
Learning: In internal/api/plugin.go (SignPluginMessages), the correct workflow is to persist the transaction via txIndexerService.CreateTx *before* policy evaluation, leaving it in the PROPOSED state; this behavior is intentional and should not be treated as an issue.

Applied to files:

  • plugin/tx_indexer/pkg/chain/thorchain.go
  • internal/api/plugin.go
📚 Learning: 2025-06-03T10:33:27.789Z
Learnt from: webpiratt
Repo: vultisig/verifier PR: 105
File: internal/tx_indexer/worker_test.go:0-0
Timestamp: 2025-06-03T10:33:27.789Z
Learning: In integration tests for the tx_indexer package, test data is intentionally not cleaned up after tests finish to allow for manual inspection of database rows. The team considers tests isolated since each test creates its own unique data. Future CI integration may involve using testify.Suite package for better test management.

Applied to files:

  • plugin/tx_indexer/pkg/chain/thorchain.go
🧬 Code graph analysis (3)
plugin/tx_indexer/pkg/config/config.go (1)
plugin/tx_indexer/pkg/rpc/thorchain.go (1)
  • THORChain (18-22)
plugin/tx_indexer/chains_list.go (2)
plugin/tx_indexer/pkg/rpc/thorchain.go (2)
  • THORChain (18-22)
  • NewTHORChain (24-40)
plugin/tx_indexer/pkg/chain/thorchain.go (1)
  • NewTHORChainIndexer (17-21)
plugin/tx_indexer/pkg/rpc/thorchain.go (1)
plugin/tx_indexer/pkg/rpc/types.go (4)
  • TxOnChainStatus (9-9)
  • TxOnChainFail (14-14)
  • TxOnChainPending (12-12)
  • TxOnChainSuccess (13-13)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 272d7a6 and b811fd7.

📒 Files selected for processing (2)
  • go.mod (1 hunks)
  • plugin/tx_indexer/pkg/rpc/thorchain.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • go.mod
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: webpiratt
Repo: vultisig/verifier PR: 156
File: tx_indexer/chains_list.go:22-37
Timestamp: 2025-06-10T19:00:10.934Z
Learning: The tx_indexer component must have every supported RPC (currently Bitcoin and Ethereum) configured; partial configuration is considered invalid and the application should fail fast if any RPC initialisation fails.
📚 Learning: 2025-06-10T18:57:04.359Z
Learnt from: webpiratt
Repo: vultisig/verifier PR: 156
File: internal/api/plugin.go:72-86
Timestamp: 2025-06-10T18:57:04.359Z
Learning: In internal/api/plugin.go (SignPluginMessages), the correct workflow is to persist the transaction via txIndexerService.CreateTx *before* policy evaluation, leaving it in the PROPOSED state; this behavior is intentional and should not be treated as an issue.

Applied to files:

  • plugin/tx_indexer/pkg/rpc/thorchain.go
🔇 Additional comments (1)
plugin/tx_indexer/pkg/rpc/thorchain.go (1)

97-98: Response body is correctly closed immediately after reading.

This properly addresses the previous review feedback about socket leaks in the retry loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add thorchain support to tx_indexer

1 participant