-
Notifications
You must be signed in to change notification settings - Fork 18
feat: create svm spoke _update #976
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
base: epic/svm-client
Are you sure you want to change the base?
Conversation
2fb3b1a
to
75a9749
Compare
Signed-off-by: james-a-morris <[email protected]>
8e9649e
to
0858448
Compare
Signed-off-by: james-a-morris <[email protected]>
Signed-off-by: james-a-morris <[email protected]>
src/clients/BaseAbstractClient.ts
Outdated
@@ -58,15 +59,21 @@ export abstract class BaseAbstractClient { | |||
* @provider Ethers RPC provider instance. | |||
* @returns An EventSearchConfig instance if valid, otherwise an UpdateFailureReason. | |||
*/ | |||
public async updateSearchConfig(provider: providers.Provider): Promise<EventSearchConfig | UpdateFailureReason> { | |||
public async updateSearchConfig( | |||
provider: providers.Provider | Rpc<SolanaRpcApiFromTransport<RpcTransport>> |
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.
Something that we can do later is to use a helper type whenever we refer to a Solana provider. We already have one (here) but I think we should rename it and maybe relocate it too since it's not just a spoke util.
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.
Yeah we can relocate it to arch/svm/types.ts
for example.
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.
const errMsg = BigNumber.isBigNumber(currentTime) | ||
? `currentTime: ${currentTime} < ${toBN(this.currentTime)}` | ||
: `currentTime is not a BigNumber: ${JSON.stringify(currentTime)}`; | ||
throw new Error(`SpokePoolClient::update: ${errMsg}`); |
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.
Should we specify that this was thrown by SvmSpokePoolClient
?
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.
if (provider instanceof providers.Provider) { | ||
toBlock = await provider.getBlockNumber(); | ||
} else { | ||
toBlock = Number(await provider.getBlockHeight({ commitment: "confirmed" }).send()); |
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.
@james-a-morris Did you check on the time delta for confirmed
vs processed
? I think the equivalent behaviour on EVM is processed
since we typically don't want to impose any finality overheads on the data ingested here.
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 delta is usually 0-2 blocks in practice. The reason for choosing confirmed
over processed
is that our events use confirmed slots
Signed-off-by: james-a-morris <[email protected]>
…Address types to strings; update SvmSpokePoolClient to utilize this function; include tests for unwrapEventData functionality
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.
Left two comments sharing some errors I had when testing locally.
); | ||
return Promise.all( | ||
events.map(async (event): Promise<SortableEvent> => { | ||
const block = await this.rpc.getBlock(event.slot).send(); |
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.
I think we have to add {maxSupportedTransactionVersion: 0}
as config for this call. This was failing locally for me with:
SolanaError: Transaction version (0) is not supported by the requesting client.
Please try the request again with the following configuration parameter: "maxSupportedTransactionVersion": 0
if (!BigNumber.isBigNumber(currentTime) || currentTime.lt(this.currentTime)) { | ||
const errMsg = BigNumber.isBigNumber(currentTime) | ||
? `currentTime: ${currentTime} < ${toBN(this.currentTime)}` | ||
: `currentTime is not a BigNumber: ${JSON.stringify(currentTime)}`; |
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.
I'm also getting an error here: TypeError: Do not know how to serialize a BigInt
.
…res; add isUint8Array utility function; update SvmSpokePoolClient for improved transaction hash handling; include new tests for FillUtils
Signed-off-by: james-a-morris <[email protected]>
WIP (not working atm)