Fix "Top Hat ID is invalid" error that pops up right after new role creation is executed #2103
Conversation
… adding that to state
✅ Deploy Preview for decent-interface-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -79,7 +80,7 @@ const useHatsTree = () => { | |||
| } | |||
| } | |||
| } catch (e) { | |||
| setHatsTree(undefined); | |||
| setHatsTree(null); | |||
There was a problem hiding this comment.
This is a sub-fix. The real fix is to not try to fetch from subgraph immediately after receiving event. Give Subgraph a few seconds to index the new Hat Tree.
But, even if we do get to it too early, let's set the hat Tree to null not undefined, so the loading screen goes away and it's as if the Safe has no Roles (which is technically accurate here because the given Tree ID doesn't line up with a valid Tree in the shape that we want (or rather, we can't get it from Subgraph).
| const topHatIdEvent = events | ||
| .filter(event => event.args.key && event.args.key === 'topHatId') | ||
| .pop(); | ||
|
|
||
| if (!hatsTreeIdEvent) { | ||
| if (!topHatIdEvent) { | ||
| return null; | ||
| } | ||
|
|
||
| if (!hatsTreeIdEvent.args.value) { | ||
| if (!topHatIdEvent.args.value) { | ||
| logError({ | ||
| message: "KVPairs 'hatsTreeIdEvent' without a value", | ||
| message: "KVPairs 'topHatIdEvent' without a value", | ||
| network: chainId, | ||
| args: { | ||
| transactionHash: hatsTreeIdEvent.transactionHash, | ||
| logIndex: hatsTreeIdEvent.logIndex, | ||
| transactionHash: topHatIdEvent.transactionHash, | ||
| logIndex: topHatIdEvent.logIndex, | ||
| }, | ||
| }); | ||
| return undefined; | ||
| } | ||
|
|
||
| try { | ||
| const topHatId = BigInt(hatsTreeIdEvent.args.value); | ||
| const topHatId = BigInt(topHatIdEvent.args.value); | ||
| const treeId = hatIdToTreeId(topHatId); | ||
| return treeId; | ||
| } catch (e) { | ||
| logError({ | ||
| message: "KVPairs 'hatsTreeIdEvent' value not a number", | ||
| message: "KVPairs 'topHatIdEvent' value not a number", | ||
| network: chainId, | ||
| args: { | ||
| transactionHash: hatsTreeIdEvent.transactionHash, | ||
| logIndex: hatsTreeIdEvent.logIndex, | ||
| transactionHash: topHatIdEvent.transactionHash, | ||
| logIndex: topHatIdEvent.logIndex, | ||
| }, |
| onLogs: logs => { | ||
| // dev: when this event is captured in realtime, give the subgraph | ||
| // time to index, and do that most cleanly by not even telling the rest | ||
| // of our code that we have the hats tree id until some time has passed. | ||
| setTimeout(() => { | ||
| setHatsTreeId(getHatsTreeId(logs, chain.id)); | ||
| }, 10_000); | ||
| }, |
There was a problem hiding this comment.
Here's the good stuff. If we're getting the new Hat Tree ID from a live event, then pause 10 seconds before setting the hats tree id in state. Setting hats tree id in state is what triggers fetching the Tree from Subgraph.
| // time to index, and do that most cleanly by not even telling the rest | ||
| // of our code that we have the hats tree id until some time has passed. | ||
| setTimeout(() => { | ||
| setHatsTreeId(getHatsTreeId(logs, chain.id)); |
There was a problem hiding this comment.
I assume that's not the case but just wanna make sure - won't it prevent loading existing tree and not the one that just created from appearing for 10 secs?
There was a problem hiding this comment.
Yeah this timeout is just for when capturing events from the live event listener. It doesn't affect loading data from past events like will be the majority of loads
| onLogs: logs => { | ||
| // dev: when this event is captured in realtime, give the subgraph | ||
| // time to index, and do that most cleanly by not even telling the rest | ||
| // of our code that we have the hats tree id until some time has passed. | ||
| setTimeout(() => { | ||
| setHatsTreeId(getHatsTreeId(logs, chain.id)); | ||
| }, 10_000); | ||
| }, |
adamgall
merged commit 848f79e
into
roles-0.1.1/desktop-and-other-updates
Closes #2083
After hearing live event for new Hat Tree Id, pause 10 seconds before adding that to state