Skip to content

Remove Distributor from RPC service#41

Merged
ordishs merged 18 commits into
bsv-blockchain:mainfrom
shruggr:feature/22-remove-distributor
Oct 29, 2025
Merged

Remove Distributor from RPC service#41
ordishs merged 18 commits into
bsv-blockchain:mainfrom
shruggr:feature/22-remove-distributor

Conversation

@shruggr

@shruggr shruggr commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

I may have gotten a little ambitious with this one, but I was already largely there with #35, and my AI already had all the context. Abstraction of the distributor has been implemented here, and I'm happy to transfer the repo if you want it: https://github.com/shruggr/teranode-tx-distributor

Summary

Complete removal of the Distributor component from the RPC service. The RPC service now uses the propagation client directly for transaction submission.

The Distributor has been extracted to a separate repository for continued development: https://github.com/shruggr/teranode-tx-distributor

Changes

  • Delete Distributor.go and all Distributor tests (removed ~1,000 lines of code)
  • Remove Distributor from test daemon setup
  • Remove Distributor-related settings (DistributorTimeout, DistributorFailureTolerance)
  • Update all test files to use propagation client directly
  • Update PlantUML diagrams to reflect new architecture

Related Issues

Addresses #22

Dependencies

Builds on #33 which introduced the propagation client usage in RPC.

shruggr and others added 13 commits October 21, 2025 22:07
Changes sendrawtransaction RPC handler to return transaction ID as a string
instead of ResponseWrapper array. Adds synchronous validation by calling
validator service directly before returning, ensuring invalid transactions
are rejected immediately per Bitcoin RPC spec.
This updates the RPC service to use the propagation service client
directly instead of going through the Distributor, addressing issue bsv-blockchain#33.

Changes:
- Extract propagation.Client interface for testability
- Update RPC Server to accept propagation client
- Modify sendrawtransaction handler to use propagation client
- Update all RPC tests to work with new propagation client mock

This takes direction from issue bsv-blockchain#22 but does not fully implement it yet.

Fixes bsv-blockchain#33
…tion

The RPC service now creates its own propagation client internally,
so it no longer needs txStore and validatorClient passed in from daemon.
The RPC service now uses the propagation client directly for transaction
submission. The Distributor component is no longer needed and has been
removed along with its tests and configuration settings.

Changes:
- Delete Distributor.go and all Distributor tests
- Remove Distributor from test daemon setup
- Remove Distributor-related settings (DistributorTimeout, DistributorFailureTolerance)
- Update all test files to use propagation client directly
- Update PlantUML diagrams to reflect new architecture

Addresses bsv-blockchain#22
@ordishs ordishs self-requested a review October 29, 2025 15:07
@github-actions

github-actions Bot commented Oct 29, 2025

Copy link
Copy Markdown
Contributor

🤖 Claude Code Review

Status: Complete


Summary:

This PR successfully removes the Distributor component (~1,500 lines) and simplifies transaction submission by using the propagation client directly. The changes are clean and well-structured.

Findings: No issues found.

The architectural simplification is sound:

  • Removed Distributor layer that added complexity without clear benefit
  • RPC now uses propagation client directly via ProcessTransaction
  • Test daemon properly updated to use propagation client
  • Settings cleaned up (removed DistributorTimeout, DistributorFailureTolerance)
  • All test files consistently updated across e2e, docker, and unit tests
  • PlantUML diagrams updated to reflect new architecture

The new ClientInterface abstraction in propagation service provides a clean contract for future implementations and testing scenarios.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)
C Reliability Rating on New Code (required ≥ A)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@ordishs ordishs merged commit b37c84d into bsv-blockchain:main Oct 29, 2025
8 checks passed
oskarszoon added a commit to oskarszoon/teranode that referenced this pull request May 11, 2026
Drop the dangling reference to the regression test (removed in the
previous commit) and the overspecified production-tx details. Keep the
two pieces of information a future bumper actually needs: which BDK PR
landed the hot-fix, and the constraint that v1.2.4 should not be
adopted until bitcoin-sv/bdk PR bsv-blockchain#41 has merged.
oskarszoon added a commit that referenced this pull request May 11, 2026
…he (#842)

Pins github.com/bitcoin-sv/bdk/module/gobdk to a pseudo-version of the
hot-fix on top of v1.2.3 (bitcoin-sv/bdk PR #40) that adds a per-instance
CachingScriptChecker overriding CheckSig to short-circuit identical
signature verifications within one EvalScript run.

Without the hot-fix the validator stalls for hours on testnet block
1,451,505 / tx 7bc9a3408dd0c87b835c887a0bce22c20788fc3c4b953929d4367656d80acab5,
whose 490 KB locking script performs 245,001 identical ECDSA verifications.
The cgo entry point is uninterruptible from Go so it presents to operators
as a hang in _Cfunc_ScriptEngine_VerifyScript. With the hot-fix the
verifier completes in ~70 s.

Do not bump gobdk to v1.2.4 (or later) until bitcoin-sv/bdk PR #41 — the
master/v1.2.4 port of the same hot-fix — has been merged.
@oskarszoon oskarszoon mentioned this pull request May 12, 2026
4 tasks
icellan added a commit that referenced this pull request May 18, 2026
Conflict: go.mod — main bumped gobdk to v1.2.5-... in #839. Took main's
newer version and dropped the obsolete "DO NOT bump to v1.2.4" comment,
since PR #41 has now merged. The PR's BSV aerospike-client fork line
was preserved alongside.
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.

3 participants