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: update for API route change #47

Merged
merged 5 commits into from
Jun 6, 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
8 changes: 6 additions & 2 deletions ape_safe/_cli/safe_mgmt.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,16 @@ def remove(cli_ctx: SafeCliContext, safe, skip_confirmation):

@click.command(cls=ConnectedProviderCommand)
@safe_cli_ctx()
@click.argument("address", type=ChecksumAddress)
@click.argument("account")
@click.option("--confirmed", is_flag=True, default=None)
def all_txns(cli_ctx: SafeCliContext, address, confirmed):
def all_txns(cli_ctx: SafeCliContext, account, confirmed):
"""
View and filter all transactions for a given Safe using Safe API
"""
if account in cli_ctx.account_manager.aliases:
Copy link
Member

Choose a reason for hiding this comment

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

this logic seems argument-callback worthy

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes but I think could be upstreamed to ape.cli

account = cli_ctx.account_manager.load(account)

address = cli_ctx.conversion_manager.convert(account, AddressType)

# NOTE: Create a client to support non-local safes.
client = cli_ctx.safes.create_client(address)
Expand Down
16 changes: 9 additions & 7 deletions ape_safe/client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,22 +77,24 @@ def get_next_nonce(self) -> int:

def _all_transactions(self) -> Iterator[SafeApiTxData]:
"""
confirmed: Confirmed if True, not confirmed if False, both if None
Get all transactions from safe, both confirmed and unconfirmed
"""

url = f"safes/{self.address}/transactions"
url = f"safes/{self.address}/all-transactions"
while url:
response = self._get(url)
data = response.json()

for txn in data.get("results"):
# TODO: Replace with `model_validate()` after ape 0.7.
# NOTE: Using construct because of pydantic v2 back import validation error.
if "isExecuted" in txn and txn["isExecuted"]:
yield ExecutedTxData.model_validate(txn)
if "isExecuted" in txn:
antazoey marked this conversation as resolved.
Show resolved Hide resolved
if txn["isExecuted"]:
yield ExecutedTxData.model_validate(txn)

else:
yield UnexecutedTxData.model_validate(txn)
else:
yield UnexecutedTxData.model_validate(txn)

# else it is an incoming transaction

url = data.get("next")

Expand Down
14 changes: 9 additions & 5 deletions ape_safe/client/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ def get_transactions(
elif confirmed and not is_confirmed:
continue # NOTE: Skip not confirmed transactions

if txn.nonce < next_nonce and isinstance(txn, UnexecutedTxData):
# NOTE: use `type(txn) is ...` because ExecutedTxData is a subclass of UnexecutedTxData
if txn.nonce < next_nonce and type(txn) is UnexecutedTxData:
continue # NOTE: Skip orphaned transactions

if filter_by_ids and txn.safe_tx_hash not in filter_by_ids:
Expand Down Expand Up @@ -136,10 +137,13 @@ def _http(self):
return urllib3.PoolManager(ca_certs=certifi.where())

def _request(self, method: str, url: str, json: Optional[dict] = None, **kwargs) -> Response:
# **WARNING**: The trailing slash in the URL is CRITICAL!
# If you remove it, things will not work as expected.

api_url = f"{self.transaction_service_url}/api/v1/{url}/"
# NOTE: paged requests include full url already
if url.startswith(f"{self.transaction_service_url}/api/v1/"):
api_url = url
else:
# **WARNING**: The trailing slash in the URL is CRITICAL!
# If you remove it, things will not work as expected.
api_url = f"{self.transaction_service_url}/api/v1/{url}/"
do_fail = not kwargs.pop("allow_failure", False)

# Use `or 10` to handle when None is explicit.
Expand Down
Loading