-
Notifications
You must be signed in to change notification settings - Fork 2
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
Integrate Tenderly to local_web3 #436
Conversation
WalkthroughThe changes introduce a new optional configuration variable Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Tenderly
participant Web3
participant LocalChain
User->>Web3: Request local web3 instance
Web3->>Tenderly: Check for Tenderly RPC availability
alt Tenderly RPC available
Web3->>Tenderly: Create Web3 instance
Web3->>Tenderly: Fund accounts with xDai
else Tenderly RPC not available
Web3->>LocalChain: Fallback to Foundry network
end
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 (
|
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
prediction_market_agent_tooling/config.py (1)
38-38
: LGTM! Consider adding a property method and documentation.The addition of
TENDERLY_FORK_RPC
is consistent with the existing code style and aligns with the PR objective of integrating Tenderly. However, for improved consistency and clarity, consider the following suggestions:
Add a property method for
TENDERLY_FORK_RPC
, similar to other variables in this class. This would allow for any necessary checks or transformations.Add a docstring to explain the purpose and expected format of this configuration option.
Here's a suggested implementation:
TENDERLY_FORK_RPC: t.Optional[str] = None @property def tenderly_fork_rpc(self) -> t.Optional[str]: """ Returns the Tenderly fork RPC endpoint if configured. This should be a valid HTTP or HTTPS URL pointing to a Tenderly fork RPC endpoint. If not set, it returns None, indicating that Tenderly integration is not configured. """ return self.TENDERLY_FORK_RPCThis addition would maintain consistency with other properties in the class and provide clear documentation for users of this configuration.
tests_integration_with_local_chain/conftest.py (1)
31-46
: ReplaceConsider using the
logging
module instead ofApply this diff to implement logging:
+import logging ... @pytest.fixture(scope="session") def local_web3( load_env: None, chain: ChainManager, accounts: list[TestAccount] ) -> t.Generator[Web3, None, None]: + logger = logging.getLogger(__name__) - print("entering fixture local_web3") + logger.debug("Entering fixture local_web3") if (tenderly_fork_rpc := APIKeys().TENDERLY_FORK_RPC) is not None: - print("using tenderly rpc") + logger.info("Using Tenderly RPC") w3 = Web3(Web3.HTTPProvider(tenderly_fork_rpc)) - print("funding test accounts on tenderly") + logger.info("Funding test accounts on Tenderly") fund_account_on_tenderly( tenderly_fork_rpc, [a.address for a in accounts], xdai_type(1000) ) yield w3 else: - print("using foundry") + logger.info("Using Foundry") with chain.network_manager.parse_network_choice( "gnosis:mainnet_fork:foundry" ) as provider: w3 = Web3(Web3.HTTPProvider(provider.http_uri)) yield w3 - print("exiting fixture local_web3") + logger.debug("Exiting fixture local_web3")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- prediction_market_agent_tooling/config.py (1 hunks)
- tests_integration_with_local_chain/conftest.py (3 hunks)
🔇 Additional comments (2)
tests_integration_with_local_chain/conftest.py (2)
71-72
: Verify the correctness of thetenderly_setBalance
JSON-RPC methodEnsure that the
tenderly_setBalance
method is correctly named and supported by the Tenderly RPC API. Any mismatches could result in failures when funding accounts.
28-29
: Update dependent fixtures and tests to include the newaccounts
parameterThe
local_web3
fixture now accepts an additional parameteraccounts: list[TestAccount]
. Ensure that all fixtures and tests that depend onlocal_web3
are updated to pass theaccounts
argument to prevent any runtime errors.Run the following script to identify usages of
local_web3
that may need updating:✅ Verification successful
Verification Successful: No usages of
local_web3
found in the test suite. No updates to fixtures or tests are necessary.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all usages of `local_web3` in the test suite. rg --type py --word-regex '\blocal_web3\b' tests/Length of output: 495
Script:
#!/bin/bash # Description: Find all usages of `local_web3` in the test suite using the correct flag. rg --type py --word-regexp '\blocal_web3\b' tests/Length of output: 52
def fund_account_on_tenderly( | ||
fork_rpc: str, addresses: list[HexAddress], balance: xDai | ||
) -> None: | ||
payload = { | ||
"jsonrpc": "2.0", | ||
"method": "tenderly_setBalance", | ||
"params": [addresses, f"0x{xdai_to_wei(balance):X}"], | ||
} | ||
response = requests.post(fork_rpc, json=payload) | ||
response.raise_for_status() |
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.
Add error handling for the Tenderly account funding process
To enhance robustness, consider adding error handling to manage potential exceptions or errors when making the HTTP request to fund accounts on Tenderly. This ensures that any issues are caught and appropriately handled.
Apply this diff to improve error handling:
def fund_account_on_tenderly(
fork_rpc: str, addresses: list[HexAddress], balance: xDai
) -> None:
payload = {
"jsonrpc": "2.0",
"method": "tenderly_setBalance",
"params": [addresses, f"0x{xdai_to_wei(balance):X}"],
}
response = requests.post(fork_rpc, json=payload)
response.raise_for_status()
+ result = response.json()
+ if 'error' in result:
+ raise Exception(f"Error funding accounts: {result['error']}")
📝 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.
def fund_account_on_tenderly( | |
fork_rpc: str, addresses: list[HexAddress], balance: xDai | |
) -> None: | |
payload = { | |
"jsonrpc": "2.0", | |
"method": "tenderly_setBalance", | |
"params": [addresses, f"0x{xdai_to_wei(balance):X}"], | |
} | |
response = requests.post(fork_rpc, json=payload) | |
response.raise_for_status() | |
def fund_account_on_tenderly( | |
fork_rpc: str, addresses: list[HexAddress], balance: xDai | |
) -> None: | |
payload = { | |
"jsonrpc": "2.0", | |
"method": "tenderly_setBalance", | |
"params": [addresses, f"0x{xdai_to_wei(balance):X}"], | |
} | |
response = requests.post(fork_rpc, json=payload) | |
response.raise_for_status() | |
result = response.json() | |
if 'error' in result: | |
raise Exception(f"Error funding accounts: {result['error']}") |
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.
Noob question - why do we need to support tenderly here in addition to foundry? As I understand it, tenderly is for running solidity tests on a mock chain. What's the use of that in this repo?
Use case is if you are creating a test against local chain (foundry), but it's failing without obvious reasons. You can switch to Tenderly by providing env variable and then, you can debug your test on Tenderly. |
No description provided.