-
Notifications
You must be signed in to change notification settings - Fork 56
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(svm): web3 v2, codama clients, and events retrieval #866
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Pablo Maldonado <[email protected]>
Signed-off-by: Pablo Maldonado <[email protected]>
Signed-off-by: Pablo Maldonado <[email protected]>
Signed-off-by: Pablo Maldonado <[email protected]>
Signed-off-by: Pablo Maldonado <[email protected]>
Signed-off-by: Pablo Maldonado <[email protected]>
Signed-off-by: Pablo Maldonado <[email protected]>
codama = createFromRoot(rootNodeFromAnchor(MulticallHandlerIdl as AnchorIdl)); | ||
codama.accept(renderJavaScriptVisitor(path.join(clientsPath, "MulticallHandler"))); | ||
|
||
// codama = createFromRoot(rootNodeFromAnchor(MessageTransmitterIdl as AnchorIdl)); |
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.
Pending to fix.
Signed-off-by: Pablo Maldonado <[email protected]>
* Reads all events for a specific program. | ||
*/ | ||
export async function readProgramEvents( | ||
rpc: web3.Rpc<web3.SolanaRpcApiFromTransport<any>>, |
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.
maybe replace any
with RpcTransport
?
|
||
type GetTransactionReturnType = ReturnType<GetTransactionApi["getTransaction"]>; | ||
|
||
type GetSignaturesForAddressConfig = Readonly<{ |
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.
maybe can infer this type without replicating upstream:
type GetSignaturesForAddressConfig = Parameters<GetSignaturesForAddressApi["getSignaturesForAddress"]>[1]
until?: Signature; | ||
}>; | ||
|
||
type GetSignaturesForAddressTransaction = { |
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.
maybe can also infer this:
type GetSignaturesForAddressTransaction = ReturnType<GetSignaturesForAddressApi["getSignaturesForAddress"]>[number];
* Reads events from a transaction. | ||
*/ | ||
export async function readEvents( | ||
rpc: web3.Rpc<web3.SolanaRpcApiFromTransport<any>>, |
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.
maybe replace any
with RpcTransport
?
const [pda] = await web3.getProgramDerivedAddress({ programAddress: programId, seeds: ["__event_authority"] }); | ||
eventAuthorities.set(programId, pda); | ||
|
||
const messageAccountKeys = txResult.transaction.message.accountKeys; |
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.
have you tested this with V0 transactions? does accountKeys
also include loaded addresses from ALTs in web3-v2
?
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.
this is pretty cool! I agree with @Reinis-FRP that we can be stricter on the typing but overall this makes sense: there are two distinct categories of web3 providers and we want to be able to support both so splitting by directory is reasonable
/** | ||
* Reads all events for a specific program. | ||
*/ | ||
export async function readProgramEvents( |
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.
how did you test this?
Signed-off-by: Pablo Maldonado <[email protected]>
Signed-off-by: Pablo Maldonado <[email protected]>
/** | ||
* Reads all events for a specific program. | ||
*/ | ||
export async function readProgramEventsV2( |
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.
Renamed to readProgramEventsV2
so they do not conflict with the v1 exports. I don't think this should be the final name as we want to prioritize V2 but for now I think it's fine until we start exposing this functions
@@ -239,6 +239,8 @@ export function stringifyCpiEvent(obj: any): any { | |||
return obj.toString(); | |||
} else if (BN.isBN(obj)) { | |||
return obj.toString(); | |||
} else if (typeof obj === "bigint" && obj !== 0n) { |
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.
why 0n
is excluded?
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.
Big bad, i put that during some tests and forgot about it.
Thanks for catchinng!
Signed-off-by: Pablo Maldonado <[email protected]>
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.
nice!
Changes proposed in this PR:
@solana/web3.js
v2 renamed as@solana/web3-v2.js
to support the previous version as wellCan be tested with: