Skip to content

Add sanity checks to Derive client & fix manager address handling#36

Merged
8ball030 merged 15 commits intomainfrom
feat/client-sanity-checks
May 13, 2025
Merged

Add sanity checks to Derive client & fix manager address handling#36
8ball030 merged 15 commits intomainfrom
feat/client-sanity-checks

Conversation

@Karrenbelt
Copy link
Copy Markdown
Collaborator

@Karrenbelt Karrenbelt commented May 8, 2025

  1. ensure address is in fact a correct address
  2. ensure that it is in fact a smart contract address on Derive
  3. verifies the signer is among the registered session keys of the smart contract wallet
  4. ensure that if a subaccount id is provided this matches what we retrieve from Derive
  5. selects manager address based on the account's margin type and currency

NOTE: temporarily disables "create_order" related tests; these continuously timeout, also in the v2-action-signing-python

Comment on lines +74 to +77
@validate_arguments(config=dict(arbitrary_types_allowed=True))
def __init__(
self,
wallet: str,
wallet: Address,
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This validates and type casts input (address to check-summed address, subaccount_id is str to int)

Comment on lines +827 to +831
def get_unique_manager(margin_type, currency):
matches = manager_by_type.get((margin_type, currency), [])
if len(matches) != 1:
raise ValueError(f"Expected exactly one ManagerAddress for {(margin_type, currency)}, found {matches}")
return matches[0]
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

the order changed; indexing is brittle. Also now there are 5 managers retrieved. We're selecting by margin type and currency, ensuring there is exactly one unique match (as we assume)

Comment on lines +52 to +55
class MarginType(Enum):
SM = "SM"
PM = "PM"
PM2 = "PM2"
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

note that PM2 is new among the manager addresses

Comment thread pyproject.toml
[tool.poetry.group.dev.dependencies]
pytest = "^7.4.2"
black = "^23.10.1"
black = "^24"
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

otherwise this throws since <24 doesn't know of / deal with py313

target-version = ['py310', 'py311', 'py312', 'py313']

@Karrenbelt Karrenbelt changed the title Add sanity checks to Derive client: address, wallet, ownership, subaccount Add sanity checks to Derive client & fix manager address handling May 8, 2025
@Karrenbelt Karrenbelt requested a review from 8ball030 May 12, 2025 23:13
@8ball030 8ball030 merged commit 736b014 into main May 13, 2025
1 check passed
@8ball030 8ball030 deleted the feat/client-sanity-checks branch May 13, 2025 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants