-
Notifications
You must be signed in to change notification settings - Fork 1
Add fallback for newHeads subscription with include tx data flag #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe Changes
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # First attempt: with False flag, not all providers support this | ||
| subscription_msg: str = json.dumps( | ||
| { | ||
| "id": 1, | ||
| "jsonrpc": "2.0", | ||
| "method": "eth_subscribe", | ||
| "params": ["newHeads"], | ||
| "params": ["newHeads", False], | ||
| } | ||
| ) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider using the official includeTransactions object form instead of a bare False flag
Geth & Erigon accept the form ["newHeads", {"includeTransactions": false}].
Some providers silently ignore an unrecognised second positional argument, which means you may think you reduced payload while in fact you did not.
-"params": ["newHeads", False],
+"params": ["newHeads", {"includeTransactions": False}],This keeps the call explicit, self-documenting, and maximises provider compatibility.
📝 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.
| # First attempt: with False flag, not all providers support this | |
| subscription_msg: str = json.dumps( | |
| { | |
| "id": 1, | |
| "jsonrpc": "2.0", | |
| "method": "eth_subscribe", | |
| "params": ["newHeads"], | |
| "params": ["newHeads", False], | |
| } | |
| ) | |
| # First attempt: with False flag, not all providers support this | |
| subscription_msg: str = json.dumps( | |
| { | |
| "id": 1, | |
| "jsonrpc": "2.0", | |
| "method": "eth_subscribe", | |
| - "params": ["newHeads", False], | |
| + "params": ["newHeads", {"includeTransactions": False}], | |
| } | |
| ) |
🤖 Prompt for AI Agents
In metrics/ethereum.py around lines 182 to 191, replace the second parameter in
the "params" list from the bare False flag to the explicit object
{"includeTransactions": false}. This means changing the "params" entry from
["newHeads", False] to ["newHeads", {"includeTransactions": false}] to ensure
compatibility with providers like Geth and Erigon and to make the call explicit
and self-documenting.
| # If subscription failed, try without the False flag | ||
| if subscription_data.get("result") is None: | ||
| raise ValueError("Subscription to newHeads failed") | ||
| self.subscription_id = subscription_data["result"] | ||
| logging.warning( | ||
| "Subscription with False flag failed, retrying without flag" | ||
| ) | ||
|
|
||
| fallback_subscription_msg: str = json.dumps( | ||
| { | ||
| "id": 1, | ||
| "jsonrpc": "2.0", | ||
| "method": "eth_subscribe", | ||
| "params": ["newHeads"], | ||
| } | ||
| ) | ||
|
|
||
| await self.send_with_timeout( | ||
| websocket, fallback_subscription_msg, WS_DEFAULT_TIMEOUT | ||
| ) | ||
| fallback_response: str = await self.recv_with_timeout( | ||
| websocket, WS_DEFAULT_TIMEOUT | ||
| ) | ||
| fallback_subscription_data = json.loads(fallback_response) | ||
|
|
||
| if fallback_subscription_data.get("result") is None: | ||
| raise ValueError("Subscription to newHeads failed even without flag") | ||
|
|
||
| self.subscription_id = fallback_subscription_data["result"] | ||
| else: | ||
| self.subscription_id = subscription_data["result"] | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Two follow-ups for the fallback logic
- Re-using the same
id(1) for the second request can confuse JSON-RPC multiplexers.
Safer to increment it:
- "id": 1,
+ "id": 2,- The current check only looks at
result is None.
Providers usually reply with an"error"field on failure; when that happens you’ll silently continue thinking the subscription succeeded (becauseresultmay be an empty string or omitted).
Guard against this:
-if subscription_data.get("result") is None:
+if (subscription_data.get("result") is None) or ("error" in subscription_data):Same for the fallback response.
Addressing both points prevents false-positives and avoids “dangling” subscriptions.
📝 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.
| # If subscription failed, try without the False flag | |
| if subscription_data.get("result") is None: | |
| raise ValueError("Subscription to newHeads failed") | |
| self.subscription_id = subscription_data["result"] | |
| logging.warning( | |
| "Subscription with False flag failed, retrying without flag" | |
| ) | |
| fallback_subscription_msg: str = json.dumps( | |
| { | |
| "id": 1, | |
| "jsonrpc": "2.0", | |
| "method": "eth_subscribe", | |
| "params": ["newHeads"], | |
| } | |
| ) | |
| await self.send_with_timeout( | |
| websocket, fallback_subscription_msg, WS_DEFAULT_TIMEOUT | |
| ) | |
| fallback_response: str = await self.recv_with_timeout( | |
| websocket, WS_DEFAULT_TIMEOUT | |
| ) | |
| fallback_subscription_data = json.loads(fallback_response) | |
| if fallback_subscription_data.get("result") is None: | |
| raise ValueError("Subscription to newHeads failed even without flag") | |
| self.subscription_id = fallback_subscription_data["result"] | |
| else: | |
| self.subscription_id = subscription_data["result"] | |
| # If subscription failed, try without the False flag | |
| if (subscription_data.get("result") is None) or ("error" in subscription_data): | |
| logging.warning( | |
| "Subscription with False flag failed, retrying without flag" | |
| ) | |
| fallback_subscription_msg: str = json.dumps( | |
| { | |
| "id": 2, | |
| "jsonrpc": "2.0", | |
| "method": "eth_subscribe", | |
| "params": ["newHeads"], | |
| } | |
| ) | |
| await self.send_with_timeout( | |
| websocket, fallback_subscription_msg, WS_DEFAULT_TIMEOUT | |
| ) | |
| fallback_response: str = await self.recv_with_timeout( | |
| websocket, WS_DEFAULT_TIMEOUT | |
| ) | |
| fallback_subscription_data = json.loads(fallback_response) | |
| if (fallback_subscription_data.get("result") is None) or ("error" in fallback_subscription_data): | |
| raise ValueError("Subscription to newHeads failed even without flag") | |
| self.subscription_id = fallback_subscription_data["result"] | |
| else: | |
| self.subscription_id = subscription_data["result"] |
🤖 Prompt for AI Agents
In metrics/ethereum.py between lines 196 and 225, the fallback subscription
logic reuses the same JSON-RPC request id (1), which can cause confusion with
multiplexers; increment the id for the fallback request to a unique value. Also,
improve the failure check by verifying if the response contains an "error" field
in addition to checking if "result" is None, for both the initial and fallback
subscription responses, and raise an error if an "error" is present to avoid
false success assumptions.
|
Alchemy doesn't support this flag at all (ignoring it silently) and doesn't provide a parameter for disabling a transaction array (which is disabled by other providers by default). I will contact their support. |
|
Alchemy fixed their message structure. Cancelling this PR since there is no need in it. |
This update adds a False flag to the newHeads subscription parameters to exclude transaction details and reduce payload size. Since not all EVM providers support this flag, the code includes fallback logic that retries the subscription without the flag if the initial attempt fails.
Summary by CodeRabbit