feat: Store Forked Indexer when Publishing #724
Conversation
darunrs
force-pushed
the
store-forked-indexer
branch
from
bf26ae4
to
2aebf93
Compare
| @@ -455,44 +361,16 @@ impl Contract { | |||
| } | |||
| } | |||
|
|
|||
| pub fn list_indexer_functions(&self, account_id: Option<String>) -> OldAccountOrAllIndexers { | |||
There was a problem hiding this comment.
Can we just double check these aren't used anywhere in the codebase (probably just frontend) to ensure this isn't a breaking change.
There was a problem hiding this comment.
So we do not use it anywhere. But, Gabe's account status component does use it. However, his component is only interested in the 2D map structure. He picks the Map corresponding to the account, and then gets the keys from the inner map. So, that's the reason I still kept this API. I just modified it to use our new structures, and the unit tests interestingly all passed with minimal changes.
registry/contract/src/lib.rs
Outdated
| @@ -281,7 +280,7 @@ impl Contract { | |||
| affected_account_id, | |||
| .. | |||
| } => { | |||
| if affected_account_id == "*" { | |||
| if affected_account_id.contains("*") { | |||
There was a problem hiding this comment.
This would prevent *.something.near which isn't what we want? I think we still want == "*"?
There was a problem hiding this comment.
I see. Yeah in Block Streamer, we actually have a hardcoded check where if the filter contains any of ["*", "*.near", "*.kaiching", "*.tg"] then we skip lake filtering. But the contract only checks Owner role for *. Even though *.near is basically just *.
I guess I can for now I can make them match. We'll just need to remember to evolve the contract if we make changes to the restrictions in the future.
registry/contract/src/lib.rs
Outdated
| let account_id = env::signer_account_id(); | ||
| let account_id = match account_id { | ||
| Some(account_id) => { | ||
| self.assert_roles(vec![Role::Owner]); |
There was a problem hiding this comment.
Maybe we should only make this assertion if env::signer_account_id is different from what's passed 🤔
There was a problem hiding this comment.
I think we usually don't provide it at all. At least, the frontend omits it. But I can add a condition over the assert to only assert if it's different. Not too hard to do.
| @@ -14,123 +14,6 @@ use serde::{Deserialize, Serialize}; | |||
|
|
|||
| type FunctionName = String; | |||
|
|
|||
| #[derive(Clone, Debug, Serialize, Deserialize, BorshSerialize, BorshDeserialize, PartialEq, Eq)] | |||
There was a problem hiding this comment.
These types are shared across block-streamer and coordinator, can you try compile those just to ensure they still work. I'm pretty sure there will be no impact, just want to sanity check.
There was a problem hiding this comment.
Thanks for the call out. I did a cargo build on both and they succeeded building. I don't believe I need to do any runtime checks right?
There was a problem hiding this comment.
Yup, compile check is enough.
darunrs
force-pushed
the
store-forked-indexer
branch
from
753cb0c
to
96969a1
Compare
Maintaining what Indexer was forked during development is useful for various usage statistics. I've updated the frontend to now store what indexer was forked from and store that information in the contract after publishing. We specifically store the indexer which was directly forked.
With this information, we can construct a graph using the "forked_from" as edge information, and calculate various other statistics such as how many indexers used some other indexer as a base by doing path counts. This can be done by creating an indexer which indexes the QueryApi contract.
There are some bugs in the workflow currently. Specifically, refreshing while in the forked page will cause the loss of whatever code was written. You need to re-fork the Indexer. While this benefits to ensuring the forked from field is correct, if we end up fixing this behavior, we need to then ensure forked from is stored persistently.
As part of updating the contract, I've removed various old functions which are no longer necessary.