Skip to content
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

fix: handle result from make_request() #78

Merged
merged 3 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,7 @@ jobs:
- name: Run Tests
run: pytest -m "not fuzzing" -s --cov=src -n auto
env:
WEB3_ETHEREUM_MAINNET_ALCHEMY_API_KEY: ${{ secrets.WEB3_ETHEREUM_MAINNET_ALCHEMY_API_KEY }}
WEB3_ETHEREUM_SEPOLIA_ALCHEMY_API_KEY: ${{ secrets.WEB3_ETHEREUM_SEPOLIA_ALCHEMY_API_KEY }}
WEB3_ARBITRUM_MAINNET_ALCHEMY_API_KEY: ${{ secrets.WEB3_ARBITRUM_MAINNET_ALCHEMY_API_KEY }}
WEB3_ARBITRUM_SEPOLIA_ALCHEMY_API_KEY: ${{ secrets.WEB3_ARBITRUM_SEPOLIA_ALCHEMY_API_KEY }}
WEB3_BASE_MAINNET_ALCHEMY_API_KEY: ${{ secrets.WEB3_BASE_MAINNET_ALCHEMY_API_KEY }}
WEB3_BASE_SEPOLIA_ALCHEMY_API_KEY: ${{ secrets.WEB3_BASE_SEPOLIA_ALCHEMY_API_KEY }}
WEB3_OPTIMISM_MAINNET_ALCHEMY_API_KEY: ${{ secrets.WEB3_OPTIMISM_MAINNET_ALCHEMY_API_KEY }}
WEB3_OPTIMISM_SEPOLIA_ALCHEMY_API_KEY: ${{ secrets.WEB3_OPTIMISM_SEPOLIA_ALCHEMY_API_KEY }}
WEB3_POLYGON_MAINNET_ALCHEMY_API_KEY: ${{ secrets.WEB3_POLYGON_MAINNET_ALCHEMY_API_KEY }}
WEB3_POLYGON_AMOY_ALCHEMY_API_KEY: ${{ secrets.WEB3_POLYGON_AMOY_ALCHEMY_API_KEY }}
WEB3_POLYGON_ZKEVM_MAINNET_ALCHEMY_API_KEY: ${{ secrets.WEB3_POLYGON_ZKEVM_MAINNET_ALCHEMY_API_KEY }}
WEB3_POLYGON_ZKEVM_CARDONA_ALCHEMY_API_KEY: ${{ secrets.WEB3_POLYGON_ZKEVM_CARDONA_ALCHEMY_API_KEY }}
WEB3_ALCHEMY_PROJECT_ID: ${{ secrets.WEB3_ALCHEMY_PROJECT_ID }}

# NOTE: uncomment this block after you've marked tests with @pytest.mark.fuzzing
# fuzzing:
Expand Down
6 changes: 3 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,18 @@ repos:
- id: isort

- repo: https://github.com/psf/black
rev: 24.4.2
rev: 24.8.0
hooks:
- id: black
name: black

- repo: https://github.com/pycqa/flake8
rev: 7.1.0
rev: 7.1.1
hooks:
- id: flake8

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.10.1
rev: v1.11.1
hooks:
- id: mypy
additional_dependencies: [types-PyYAML, types-requests, pydantic, types-setuptools]
Expand Down
7 changes: 6 additions & 1 deletion ape_alchemy/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def create_access_list(
def make_request(self, rpc: str, parameters: Optional[Iterable] = None) -> Any:
parameters = parameters or []
try:
return self.web3.provider.make_request(RPCEndpoint(rpc), parameters)
result = self.web3.provider.make_request(RPCEndpoint(rpc), parameters)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't want this result, I'll approve

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the result is returned later on after some processing!

except HTTPError as err:
response_data = err.response.json() if err.response else {}
if "error" not in response_data:
Expand All @@ -203,6 +203,11 @@ def make_request(self, rpc: str, parameters: Optional[Iterable] = None) -> Any:
)
raise cls(message) from err

if isinstance(result, dict) and (res := result.get("result")):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bugfix here, does the same as ape-ethereum, thus causing eth_call (and other RPCs) to function correctly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnson2427 it processes it here (like ape core does) and returns the extracted value

this was kind of a regression because as part of the optimizations in core, switched the base class to use make_request more, causing us to reach this bug

return res

return result

def send_private_transaction(self, txn: TransactionAPI, **kwargs) -> ReceiptAPI:
"""
See `Alchemy's guide <https://www.alchemy.com/overviews/ethereum-private-transactions>`__
Expand Down
6 changes: 3 additions & 3 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
"websocket-client", # Used for web socket integration testing
],
"lint": [
"black>=24.4.2,<25", # Auto-formatter and linter
"mypy>=1.10.1,<2", # Static type analyzer
"black>=24.8.0,<25", # Auto-formatter and linter
"mypy>=1.11.1,<2", # Static type analyzer
"types-setuptools", # Needed for mypy type shed
"types-requests", # Needed for mypy type shed
"flake8>=7.1.0,<8", # Style linter
"flake8>=7.1.1,<8", # Style linter
"flake8-breakpoint>=1.1.0,<2", # Detect breakpoints left in code
"flake8-print>=5.0.0,<6", # Detect print statements left in code
"isort>=5.13.2,<6", # Import sorting linter
Expand Down
15 changes: 15 additions & 0 deletions tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,18 @@ def test_polygon_zkevm():
tx = provider.network.ecosystem.create_transaction(receiver=receiver)
with pytest.raises(APINotImplementedError):
_ = provider.create_access_list(tx)


def test_make_requeset_handles_result():
"""
There was a bug where eth_call because ape-alchemy wasn't
handling the result from make_request properly.
"""
tx = {
"to": "0x5576815a38A3706f37bf815b261cCc7cCA77e975",
"value": "0x0",
"data": "0x70a082310000000000000000000000005576815a38a3706f37bf815b261ccc7cca77e975",
}
with networks.polygon_zkevm.cardona.use_provider("alchemy") as provider:
result = provider.make_request("eth_call", [tx, "latest"])
assert not isinstance(result, dict)
Loading