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

Question / Feature: account:Account param in certain util methods may only need SA address #67

Closed
livingrockrises opened this issue Sep 25, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@livingrockrises
Copy link

livingrockrises commented Sep 25, 2024

Describe the feature you would like

below methods

  client,
  account,
  permissionId,
}: {
  client: PublicClient
  account: Account
  permissionId: Hex
}) => {
  return (await client.readContract({
    address: SMART_SESSIONS_ADDRESS,
    abi,
    functionName: 'getNonce',
    args: [permissionId, account.address],
  })) as bigint
}
export const isSessionEnabled = async ({
  client,
  account,
  permissionId,
}: {
  client: PublicClient
  account: Account
  permissionId: Hex
}) => {
  return (await client.readContract({
    address: SMART_SESSIONS_ADDRESS,
    abi,
    functionName: 'isSessionEnabled',
    args: [permissionId, account.address],
  })) as boolean
}

requires a whole Account object to be passed. can it be just smartAccountAddress or conditional optional params

Additional context

No response

@livingrockrises livingrockrises added the enhancement New feature or request label Sep 25, 2024
@kopy-kat
Copy link
Member

kopy-kat commented Oct 3, 2024

tbh I dont like how we handle accounts and clients in the sdk now - for v0.2 I have some ideas for how we can streamline the ux and open to feedback here

@Dhruv-Mishra
Copy link
Contributor

This extends to other methods as well which do not leverage any additional property from the account object other than the address.

Should this change extend to those methods as well?

@kopy-kat
Copy link
Member

kopy-kat commented Jan 7, 2025

yeah I wonder what the best solution here is - one option would be to still use the account but make it into a class and have the methods be called on this class, like account.installModule but not sure how useful that is as well and it could become quite unwieldy to maintain

what do you think the best solution would be here?

@Dhruv-Mishra
Copy link
Contributor

Where are these methods being leveraged and are these only used in a TS environment?
To what extent do we wish to enforce type safety here? Are we willing to allow any json object with an address field, containing a valid address, to be accepted as the input for our methods or do we want the object to strictly adhere to our definition of Account and Address?

Since TS types and interfaces don’t exist at runtime, if type safety is a priority, then we should either use a class or use Typeguards for our function arguments.

I will review any similar or related projects to see what they follow

@kopy-kat
Copy link
Member

kopy-kat commented Jan 8, 2025

@kopy-kat kopy-kat closed this as completed Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants