Dynamic Contract Pre Registration#252
Conversation
JonoPrest
force-pushed
the
jp/contract-preregistration
branch
from
3ad7e16 to
952732b
Compare
|
It doesn't break anything on the hosted service. Will need to adapt the UI a bit to represent the pre registration step. |
| type eventConfig<'eventFilter> = { | ||
| wildcard?: bool, | ||
| eventFilters?: SingleOrMultiple.t<'eventFilter>, | ||
| preRegister?: bool, |
There was a problem hiding this comment.
| preRegister?: bool, | |
| shouldPreRegisterDynamicContracts?: bool, |
(and everywhere else)
I think it is good to have consistent naming on this. I assume you want it to be less verbose in the config and more verbose in the code. Maybe then we just use preregister everywhere instead of the longer variable. I don't think it is unclear to just be preRegister
There was a problem hiding this comment.
IMO I prefer the verbose naming since it's quite an esoteric feature.
The shortened version I just used for the user API since I don't think they should have such a verbose flag. In the same way, we use isWildcard for our internal config everywhere but for the user API it's simply wildcard
I would be open to renaming the the user facing config flag but would prefer to keep the verbose name everywhere else.
There was a problem hiding this comment.
Cool, maybe middle ground - preRegisterDynamicContracts everywhere?
I don't really agree that the variable will be confusing, especially since the variable comes from the user's handlers and will be in our box, etc., but I think we can make it more descriptive
There was a problem hiding this comment.
@JasoonS for the user API we have Contract.Event.contractRegister so maybe even preRegisterContracts I was thinking?
Still prefer verbose naming with "should" or "is" prefix in internal code to imply boolean type.
|
|
||
| //Instantiate each time to add new registered contract addresses | ||
| let recieptsSelection = getRecieptsSelection(~contractAddressMapping, ~shouldApplyWildcards) | ||
| let recieptsSelection = if isPreRegisteringDynamicContracts { |
There was a problem hiding this comment.
Hmm, is there no way to catch this at codegen time? I guess the user will get notified as soon as it starts. So, fine.
There was a problem hiding this comment.
nvm - it is temprorary too.
There was a problem hiding this comment.
No since the config is set with the js API
JasoonS
left a comment
JasoonS
left a comment
There was a problem hiding this comment.
Cool, I think the only real thing is to change the variable to pre-register everywhere, even internally. The code seems really good. I didn't notice any bugs or anything.
It is quite a bit to follow though, so I don't fully trust myself to have caught bugs - but I think we can start testing it in the wild.
| //On the first time we enter the reorg threshold, copy all entities to entity history | ||
| //And set the isInReorgThreshold isInReorgThreshold state to true | ||
| dispatchAction(SetIsInReorgThreshold(true)) | ||
| //TODO: persist the isInReorgThreshold state to the db in a transaction with copy |
There was a problem hiding this comment.
It's implemented 👍🏼
| buffered={latestFetchedBlockNumber - firstEventBlockNumber} | ||
| outOf={toBlock - firstEventBlockNumber} | ||
| loadingColor=Secondary | ||
| loadingColor={isPreRegisteringDynamicContracts ? Primary : Secondary} |
| chainWorker: module(ChainWorker.S), | ||
| } | ||
|
|
||
| let shouldPreRegisterDynamicContracts = (chainConfig: chainConfig) => { |
There was a problem hiding this comment.
I think this function name is fine, but then change the variable to just preRegister (as the user sees it)
Co-authored-by: Jason Smythe <jason@envio.dev>
JonoPrest
force-pushed
the
jp/contract-preregistration
branch
from
b4add71 to
c8418e6
Compare
* Wip initial impl of dynamic contract pre-fetching * Fix contractPreRegistration LogSelection * Add latest processed blocks during pre registration * Add restart resistance to pre registration * Handle multiple partitions with contract preregistration * Add runtime error for pre registration on fuel and rpc * Implement user api for pre registration * Remove logs * Update TUI * Fix tests * Cleaner shouldPreRegisterDynamicContracts function Co-authored-by: Jason Smythe <jason@envio.dev> * Update naming of preRegisterDynamicContracts api --------- Co-authored-by: Jason Smythe <jason@envio.dev>
2 participants
Implements Dynamic Contract Preregistration
Before this gets merged for a release, we need to validate how the change affects the hosted service UI since events sync state table has been updated