feat/refactor: Multi-Provider improvements #842
Conversation
|
Initially it was instantiated from an event. The reason I changed it to be instantiated from a receipt was to preserve blockheight, and other tx confirmation info. Does this PR preserve those? |
anna-carroll
force-pushed
the
anna/multi-provider
branch
from
a5d27b2 to
4a6d6fa
Compare
|
you know what, i will just pass in the event and the transaction receipt |
typescript/optics-provider/src/optics/messages/OpticsMessage.ts
Outdated
Show resolved
Hide resolved
anna-carroll
changed the title
typescript/optics-provider/src/optics/messages/BridgeMessage.ts
Outdated
Show resolved
Hide resolved
typescript/optics-provider/src/optics/messages/OpticsMessage.ts
Outdated
Show resolved
Hide resolved
typescript/optics-provider/src/optics/messages/OpticsMessage.ts
Outdated
Show resolved
Hide resolved
|
Going to look over this first thing after I get some coffee, but wanted to say thanks for stepping in here. Absolutely incredible 🌸 |
| }; | ||
| } | ||
|
|
||
| export async function getRichEvents(context: OpticsContext, nameOrDomain: string | number, contract: BaseContract, logFilter: TypedEventFilter<any, any>, startBlock?: number, endBlock?: number): Promise<Array<RichEvent>> { |
There was a problem hiding this comment.
should these be module-level functions, seems like they ought to be methods on the MultiProvider or OpticsContext?
| const POLYGON_FIRST_BLOCK = 18895794; | ||
| const POLYGON_MAX_BLOCKS = 2500; | ||
|
|
||
| export async function getPolygonEvents(context: OpticsContext, nameOrDomain: string | number, contract: BaseContract, logFilter: TypedEventFilter<any, any>, startBlock?: number, endBlock?: number): Promise<Array<Event>> { |
There was a problem hiding this comment.
proposed additional kludge:
allow user to specify an early exit after finding a single event. E.g. a singleton: bool = false argument. this allows us to prevent crawling the entire range if we're looking for a particular event
| Dispatched = 0, | ||
| Included = 1, | ||
| Relayed = 2, | ||
| Processed = 3 |
There was a problem hiding this comment.
inconsistent enum declaration. either use explicit = on all elements of both enums or on just the first element of both
typescript/optics-provider/src/optics/messages/OpticsMessage.ts
Outdated
Show resolved
Hide resolved
|
|
||
| this.context = context; | ||
| async homeUpdateEvent(): Promise<UpdateEvent | undefined> { |
There was a problem hiding this comment.
need to note in docs that this function may produce false positives, if the updater has signed an update on the committedRoot that does not include this message. these should be uncommon in practice. The only way to know for sure is to keep an indexed set of updates and leaves :(
There was a problem hiding this comment.
ah shoot, you are right.
typescript/optics-provider/src/optics/messages/OpticsMessage.ts
Outdated
Show resolved
Hide resolved
|
@anna-carroll please put me out of my misery and/or review this PR |
|
-_- |
refactor: make event queries rely on ethers type system refactor: generic Annotated type refactor: improve event cache in Optics message refactor: non-looping array concat refactor: use Annotated events for message instantiation refactor: add concurrency limit to paginated event retrieval refactor: break up annotate and rely on event.getTransactionReceipt refactor: use annotated lifecycle events throughout refactor: break out events into a folder bug: check for id, not domain bug: check for id, not domain (again) bug: check for id, not domain (again) Revert "bug: check for id, not domain (again)" This reverts commit 8304985. bug: pass through signer in fromObject bug: improper fromObject checking re: domain refactor: break trace out into its own package remove .env
prestwich
force-pushed
the
anna/multi-provider
branch
from
bcc0d37 to
35cd8f9
Compare
|
merging as it's blocking GUI work. can fix up anything in separate PRs |
| @@ -1,4 +1,10 @@ | |||
| export interface Pagination { | |||
| blocks: number; | |||
| from: number; | |||
There was a problem hiding this comment.
we could have a from block for every domain (but no real need - simpler this way)
There was a problem hiding this comment.
tangential musing, i wonder if RPC providers are actually smart about this and automatically use from the block that the contract being queried was deployed at
| domain, | ||
| receipt, | ||
| name: event.eventSignature?.split('(')[0], | ||
| event, |
There was a problem hiding this comment.
i wonder if it wouldn't be beneficial to have the annotatedEvent have all the same fields as a regular event (e.g. do ...event here) so that people can interact with it as if its a normal event (with a few extra fields)
| lastBlock = endBlock; | ||
| } | ||
| // query domain pagination limit at a time, concurrently | ||
| const callArgs = []; |
There was a problem hiding this comment.
this loop isn't used. i'll open a PR
Closes #835 #827 #815