-
Notifications
You must be signed in to change notification settings - Fork 134
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: add multi-policy sponsorships #1189
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Your org has enabled the Graphite merge queue for merging into mainAdd the label “graphite-merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
@@ -78,7 +78,7 @@ type RpcConnectionConfig = | |||
chain: Chain; | |||
chains: { | |||
chain: Chain; | |||
policyId?: string; | |||
policyId?: string | string[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can one chain have multiple policy ids?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, with the idea being that when we have gas grants you can add both the grant as well as your own policy. In general this model might be worth revisiting now that we also have multi-chain policices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea with multi-chain policies, it almost feels like we can add support for that at the top level and just have a policyId?: string[]
and start pushing folks towards that
@@ -56,15 +56,15 @@ export type AlchemyAccountsConfig = { | |||
export type Connection = { | |||
transport: AlchemyTransportConfig; | |||
chain: Chain; | |||
policyId?: string; | |||
policyId?: string | string[]; | |||
}; | |||
|
|||
type RpcConnectionConfig = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
almost feels like we need one more permutation here like the following that allows you to specify the policyId array at the top level instead of nesting it
{
chain: Chain;
chains: {
chain: Chain;
transport?: AlchemyTransport
}[],
transport: AlchemyTransport;
signerConnection?: ConnectionConfig;
policyId?: string[];
}
| {
chain: Chain;
chains: {
chain: Chain;
transport?: AlchemyTransport
}[],
transport?: never;
signerConnection: ConnectionConfig;
policyId?: string[];
}
Pull Request Checklist
yarn test
)site
folder, and guidelines for updating/adding docs can be found in the contribution guide)feat!: breaking change
)yarn lint:check
) and fix any issues? (yarn lint:write
)PR-Codex overview
This PR updates the version numbers across multiple files and modifies the
policyId
parameter to accept bothstring
andstring[]
types in several locations, enhancing flexibility in handling policy IDs.Detailed summary
VERSION
from4.5.1
to4.6.0
in:aa-sdk/core/src/version.ts
account-kit/signer/src/version.ts
account-kit/logging/src/version.ts
policyId
type fromstring
tostring | string[]
in:account-kit/infra/src/client/smartAccountClient.ts
account-kit/infra/src/middleware/gasManager.ts
account-kit/core/src/types.ts
account-kit/infra/src/middleware/gasManager.ts
documentation