Allow to accept bytes in send_xdai_to, add chainId to tx params #568
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe changes in this pull request primarily focus on the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
prediction_market_agent_tooling/tools/web3_utils.py (1)
308-312: LGTM! Consider adding test coverage.The implementation correctly handles both string and bytes inputs while maintaining backward compatibility. The conditional conversion ensures that bytes input is preserved while strings are converted as before.
Would you like me to help generate test cases covering both string and bytes inputs?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
prediction_market_agent_tooling/tools/web3_utils.py(1 hunks)
🔇 Additional comments (2)
prediction_market_agent_tooling/tools/web3_utils.py (2)
300-300: LGTM! Verify Python version compatibility.
The type annotation change correctly implements the PR objective to allow bytes input. However, the union operator | requires Python 3.10+.
✅ Verification successful
Type annotation change is compatible with project requirements
The project's pyproject.toml specifies Python version requirement as ">=3.10,<3.12", which supports the union operator | in type annotations. The change is safe to use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Python version requirement in project
fd "setup.py|pyproject.toml" | xargs cat
Length of output: 2177
300-312: Verify all callers of send_xdai_to.
Let's ensure all callers of this function are aware of and compatible with the new parameter type.
✅ Verification successful
No impact on existing callers - parameter change is backward compatible
The verification shows that the data_text parameter change to accept both str | bytes is safe and backward compatible:
- All existing calls either:
- Don't use the
data_textparameter (most cases) - Use it correctly with a string value (test_local_chain.py)
- Don't use the
- The parameter remains optional with the same default value of
None - The implementation handles both string and bytes types appropriately
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all usages of send_xdai_to
rg "send_xdai_to\(" --type python -B 2 -A 2
# Search for any documentation or examples
fd -e md -e rst | xargs rg "send_xdai_to"
Length of output: 118
Script:
#!/bin/bash
# Search for all usages of send_xdai_to without file type restriction
rg "send_xdai_to\(" -B 2 -A 2
# Search for documentation files and their content
find . -type f -name "*.md" -o -name "*.rst" | xargs grep -l "send_xdai_to" || true
# Get the implementation context
rg "def send_xdai_to" -B 2 -A 2
Length of output: 4220
Script:
#!/bin/bash
# Let's examine the actual calls to see the data_text parameter usage
rg "send_xdai_to\(" -A 6
# Get the full implementation of send_xdai_to to understand the parameter changes
rg "def send_xdai_to" -A 20
Length of output: 6790
kongzii
changed the title
| @@ -153,6 +153,9 @@ def _prepare_tx_params( | |||
| from_checksummed = Web3.to_checksum_address(tx_params_new["from"]) | |||
| tx_params_new["nonce"] = web3.eth.get_transaction_count(from_checksummed) | |||
|
|
|||
| if not tx_params_new.get("chainId"): | |||
| tx_params_new["chainId"] = web3.eth.chain_id | |||
There was a problem hiding this comment.
| tx_params_new["data"] = Web3.to_bytes(text=data_text) | ||
| tx_params_new["data"] = ( | ||
| Web3.to_bytes(text=data_text) | ||
| if not isinstance(data_text, bytes) |
There was a problem hiding this comment.
zlib will return bytes, so it can be just used here
No description provided.