feat(svmSpokeUtils): get fill deadline buffer implementation #978
Conversation
melisaguevara
force-pushed
the
mg/svm-getMaxFillDeadline
branch
from
f8f8e9c to
9cf2918
Compare
melisaguevara
changed the title
| @@ -19,6 +20,7 @@ export class SvmSpokePoolClient extends SpokePoolClient { | |||
| chainId: number, | |||
| deploymentSlot: bigint, // Using slot instead of block number for SVM | |||
| eventSearchConfig: MakeOptional<EventSearchConfig, "toBlock">, | |||
| protected programId: string, | |||
There was a problem hiding this comment.
I've added programId here because I think it's useful to have direct access to it. That said, I'm also not sure this is the best approach since svmSpokeEventsClient already resolves programId internally, and that value might differ from the one passed in here. To address that I think we can either (1) use the resolved value from svmEventsClient or (2) let the callers provide programId and pass that to svmSpokeEventsClient.
For option 1, we only need to make svmSpokeAddress a public property (here).
For option 2 we'll have to remove the resolution logic from svmSpokeEventsClient entirely and turn it into a constructor param instead.
I'm curious to hear other's thoughts about this. Personally, I'm leaning toward option 2. I think it's closer to what we have for EVM and it also improves flexibility. As an example, I had to override the program signature to test some Solana features while working in the Indexer and there was other way to do so than changing the svmSpokeEventsClient.
There was a problem hiding this comment.
@melisaguevara Is it necessary for the eventsClient to be instantiated outside of the SVMSpokePoolClient? Could we instead instantiate it entirely within the .create() static method?
There was a problem hiding this comment.
Fwiw I think it makes sense to also expose the svmSpokeAddress - it's not required to be private, and could theoretically be encapsulated in a getter fn
There was a problem hiding this comment.
Is it necessary for the eventsClient to be instantiated outside of the SVMSpokePoolClient? Could we instead instantiate it entirely within the .create() static method?
@pxrl I don't think so, I think we'll always use it through the clients. This could be a follow up if we all agree to do so.
There was a problem hiding this comment.
Fwiw I think it makes sense to also expose the svmSpokeAddress - it's not required to be private, and could theoretically be encapsulated in a getter fn
@james-a-morris Yes I saw you did that in your PR. I think that's also an option, though as an end user you would have to do svmSpokeClient.svmEventsClient.getSvmSpokeAddress(). I think it would be nice to have that directly as property of SvmSpokeClient. For now I'll follow your approach but I'm also including it as a spoke client property, we can revisit this later.
There was a problem hiding this comment.
This is how it looks for now: d7e3dd6
There was a problem hiding this comment.
@pxrl I don't think so, I think we'll always use it through the clients. This could be a follow up if we all agree to do so.
Yeah; I think it's probably cleaner to incorporate it directly into the SpokePoolClient, rather than requiring it to be supplied as an external dependency. This should make it easier to instantiate.
src/arch/svm/utils.ts
Outdated
| * @param extraSeed An optional extra seed. Defaults to 0. | ||
| * @returns The PDA for the State account. | ||
| */ | ||
| export async function getStatePda(programId: string, extraSeed = 0): Promise<Address> { |
There was a problem hiding this comment.
For consistency should programId also be Address?
| @@ -19,6 +20,7 @@ export class SvmSpokePoolClient extends SpokePoolClient { | |||
| chainId: number, | |||
| deploymentSlot: bigint, // Using slot instead of block number for SVM | |||
| eventSearchConfig: MakeOptional<EventSearchConfig, "toBlock">, | |||
| protected programId: string, | |||
There was a problem hiding this comment.
Fwiw I think it makes sense to also expose the svmSpokeAddress - it's not required to be private, and could theoretically be encapsulated in a getter fn
| @@ -209,4 +209,8 @@ export class SvmSpokeEventsClient { | |||
|
|
|||
| return events; | |||
| } | |||
|
|
|||
| public getSvmSpokeAddress(): Address { | |||
| * @param extraSeed An optional extra seed. Defaults to 0. | ||
| * @returns The PDA for the State account. | ||
| */ | ||
| export async function getStatePda(programId: Address, extraSeed = 0): Promise<Address> { |
There was a problem hiding this comment.
OOC how often is extraSeed set? I think it makes sense to have but if we don't need to use it it may be extra unneeded params
This PR implements the utility function for getting svm fill deadline buffer. That's done by fetching the Spoke Pool's State PDA.