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

feat: initialize viem package #565

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat: initialize viem package #565

wants to merge 11 commits into from

Conversation

douglance
Copy link
Contributor

No description provided.

@cla-bot cla-bot bot added the cla-signed label Dec 26, 2024
@douglance douglance requested a review from spsjvc December 26, 2024 18:13
packages/sdk-viem/src/actions.ts Outdated Show resolved Hide resolved
packages/sdk-viem/tests/deposit.test.ts Outdated Show resolved Hide resolved
parentRpcUrl,
childRpcUrl,
parentWalletClient,
childWalletClient,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need the childWalletClient here

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I would not request any client here, and just create everything within the function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

could you clarify? typically, the consumer will want control of their clients, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

If they want to keep control on their client, they can extend with actions on their own. I would expect createArbitrumClient to do every thing for me and create the clients not extend them. But it's not an issue.

packages/sdk-viem/src/createArbitrumClient.ts Outdated Show resolved Hide resolved
packages/sdk-viem/src/actions.ts Outdated Show resolved Hide resolved
packages/sdk-viem/src/createArbitrumClient.ts Show resolved Hide resolved
parentRpcUrl,
childRpcUrl,
parentWalletClient,
childWalletClient,
Copy link
Contributor

Choose a reason for hiding this comment

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

If they want to keep control on their client, they can extend with actions on their own. I would expect createArbitrumClient to do every thing for me and create the clients not extend them. But it's not an issue.

@douglance douglance requested a review from chrstph-dvx January 14, 2025 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants