Fixes broken .send() – Broadcast Transactions Working Again #201
Conversation
- Fix malformed HTTP payload in BlockstreamAPI.broadcast_tx() - Fix malformed HTTP payload in BlockstreamAPI.broadcast_tx_testnet() - Remove defunct third-party API services from dispatcher lists - Per Blockstream API specification, transaction hex should be sent as raw request body - Fixes ofek#110 and ofek#170
Summary of ChangesHello @salimbk016, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical, long-standing bug in the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request provides a critical fix for transaction broadcasting, which has been failing silently. The changes correctly adjust the payload format for the Blockstream API and remove several defunct service providers from the rotation. My review includes a high-severity comment regarding a discrepancy between the PR description and the implementation, where an unreliable service provider is retained. I've also noted a minor stylistic issue. Overall, this is a valuable contribution that restores core functionality.
bit/network/services.py
Outdated
| @classmethod | ||
| def broadcast_tx_testnet(cls, tx_hex): # pragma: no cover | ||
| r = requests.post(cls.TEST_TX_PUSH_API, data={cls.TX_PUSH_PARAM: tx_hex}, timeout=DEFAULT_TIMEOUT) | ||
| r = requests.post(cls.TEST_TX_PUSH_API, data= tx_hex, timeout=DEFAULT_TIMEOUT) |
There was a problem hiding this comment.
There's a minor stylistic issue here with an extra space before tx_hex. According to the PEP 8 style guide, there should be no space around the = sign when used for a keyword argument.
| r = requests.post(cls.TEST_TX_PUSH_API, data= tx_hex, timeout=DEFAULT_TIMEOUT) | |
| r = requests.post(cls.TEST_TX_PUSH_API, data=tx_hex, timeout=DEFAULT_TIMEOUT) |
References
- PEP 8 discourages using spaces around the '=' sign when passing keyword arguments to a function. (link)
There was a problem hiding this comment.
Thanks! I’ve pushed a commit addressing this. Could you please re-review?
There was a problem hiding this comment.
Code Review
This is an excellent and much-needed fix that addresses a long-standing critical bug in transaction broadcasting. The root cause analysis is thorough and accurate. The changes correctly fix the malformed payload for the Blockstream API and remove defunct service providers, which will resolve the silent transaction failures. I have a couple of suggestions for minor code simplification and a higher-level comment regarding the new dependency on a single API provider for future consideration.
| BROADCAST_TX_MAIN = [ | ||
| BlockchairAPI.broadcast_tx, | ||
| BlockstreamAPI.broadcast_tx, | ||
| BitcoreAPI.broadcast_tx, | ||
| SmartbitAPI.broadcast_tx, # Limit 5/minute | ||
| BlockchainAPI.broadcast_tx, | ||
| ] |
There was a problem hiding this comment.
This change correctly removes the defunct API providers, which is a necessary fix for the immediate issue. However, this makes blockstream.info the sole provider for broadcasting transactions, creating a single point of failure. If Blockstream's API becomes unavailable or rate-limits requests, transaction broadcasting will be completely broken.
While this is a significant improvement over the previous silently-failing state, for long-term resilience, it would be beneficial to either:
- Investigate and add other reliable, public broadcast endpoints.
- Implement a way for users to configure their own preferred broadcast endpoints or even connect to their own node for broadcasting.
| def broadcast_tx(cls, tx_hex): # pragma: no cover | ||
| r = requests.post(cls.MAIN_TX_PUSH_API, data={cls.TX_PUSH_PARAM: tx_hex}, timeout=DEFAULT_TIMEOUT) | ||
| r = requests.post(cls.MAIN_TX_PUSH_API, data=tx_hex, timeout=DEFAULT_TIMEOUT) | ||
| return True if r.status_code == 200 else False |
There was a problem hiding this comment.
The ternary operator True if ... else False is redundant here. The comparison r.status_code == 200 already evaluates to a boolean, which can be returned directly. This simplifies the code and improves readability.
| return True if r.status_code == 200 else False | |
| return r.status_code == 200 |
| def broadcast_tx_testnet(cls, tx_hex): # pragma: no cover | ||
| r = requests.post(cls.TEST_TX_PUSH_API, data={cls.TX_PUSH_PARAM: tx_hex}, timeout=DEFAULT_TIMEOUT) | ||
| r = requests.post(cls.TEST_TX_PUSH_API, data=tx_hex, timeout=DEFAULT_TIMEOUT) | ||
| return True if r.status_code == 200 else False |
There was a problem hiding this comment.
Similar to the broadcast_tx function, this conditional return can be simplified by returning the boolean result of the comparison directly.
| return True if r.status_code == 200 else False | |
| return r.status_code == 200 |
1 participant
Pull Request: Fix Silent Transaction Broadcast Failures in BlockstreamAPI
Title
Fix: Resolve Silent Transaction Broadcast Failures in BlockstreamAPI (#110, #170)
Description
This pull request addresses a critical bug in the
bit/network/services.pymodule that has caused silent transaction broadcast failures affecting both mainnet and testnet environments. The issue has been reported multiple times by users (Issues #110, #170, Stack Overflow discussions) but the root cause has never been properly diagnosed.Problem Summary
Users of the
bitlibrary encounter a "silent failure" when attempting to broadcast Bitcoin transactions using the standard.send()interface:.send()method returns a transaction hash or cycles through retriesRoot Cause Analysis
Investigation isolated the failure to two concurrent issues within
bit/network/services.py:Issue 1: Deprecated Service Dependencies
The library's NetworkAPI dispatcher relies on a hardcoded list of third-party Bitcoin APIs that have degraded over time:
Issue 2: Malformed HTTP Payload in BlockstreamAPI (Critical Bug)
The only operational service provider (Blockstream) was failing due to an HTTP request formatting error:
Current (Broken) Implementation:
Expected Format (per Blockstream API specification):
The Blockstream
/txendpoint expects the raw hex string directly as the HTTP POST body, not wrapped in a dictionary.Why This Causes Silent Failure:
Solution
This PR implements two targeted fixes:
Fix 1: Correct HTTP Payload Format
Modify
BlockstreamAPI.broadcast_tx()andbroadcast_tx_testnet()to send the transaction hex directly as the request body:Fix 2: Purge Defunct Services
Remove broken providers from NetworkAPI dispatcher lists to prevent latency and false-negative errors:
Verification
The fix has been tested and verified:
Related Issues & Discussion
Impact Assessment
Files Modified:
bit/network/services.py(2 method implementations, 2 dispatcher list configurations)Lines Changed: ~20 lines (minimal surgical fix)
Breaking Changes: None
Testing Recommendations:
Why Now?
Supporting Documentation & Analysis
For detailed technical analysis, comprehensive methodology, and supporting documentation, please see:
📦 Repository: github.com/salimbk016/bit-broadcast-fix
This repository documents:
The code changes in this PR implement the solution identified in that comprehensive analysis.
Additional Context
The root cause was identified through systematic debugging:
Checklist
Summary
This is a critical bugfix that:
Recommended Action: Approve and merge 🚀