`[Issue #2479]` Tie streams to hat

Da-Colon commented 4 days ago

Closes #2479

Alrighty, Goes with https://github.com/decentdao/decent-contracts/pull/128 over in the contracts.

netlify[bot] commented 4 days ago

Deploy Preview for decent-interface-dev ready!

Name	Link
Latest commit	d038852f3e4ef51e03486494e3e554810d0a38a5
Latest deploy log	https://app.netlify.com/sites/decent-interface-dev/deploys/6742b7453f2e71000873ab15
Deploy Preview	https://deploy-preview-2562.app.dev.decentdao.org
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

DarksightKellar commented 3 days ago

Whoops did not mean to approve just yet since "no QA yet". Meant to leave a comment

adamgall commented 3 days ago

@Da-Colon @fractal-framework/fractal-contracts@v1.4.2 is being published right now and should be available for consumption in ~3 mins from the time of this comment.

adamgall commented 3 days ago

Also @Da-Colon just to be clear

Until the contracts are published, did not commit the tempAbis and network addresses that I use for testing.

Why did you have new ABIs? The ABIs shouldn't have changed.

Da-Colon commented 3 days ago

Also @Da-Colon just to be clear

Until the contracts are published, did not commit the tempAbis and network addresses that I use for testing.

Why did you have new ABIs? The ABIs shouldn't have changed.

Because I'm sleep deprived

Da-Colon commented 3 days ago

In the latest commit, is there a race condition here? Seems like this code will only work correctly if and only if the KeyValuePairs data comes in (and gets stored in the map) before the updateRolesWithStreams function runs.

Is that a valid concern?

I don't think so? Since its coming in is being fetched at the same time as the hatTreeId which kicks off the retreival of that hats and setHatsTree is first called. I believe that these will correctly get set prior to setHatTree being called next.

I thought about update setHatTreeId into like setHatTreeEventData (The name is why I didn't) to set the loop first. In that I effect. I could just flip These like this for the same effect.

   setHatIdsToStreamIds(getHatIdsToStreamIds(safeEvents, sablierV2LockupLinear, chain.id));
        setHatsTreeId({
          contextChainId: chain.id,
          hatsTreeId: getHatsTreeId(safeEvents, chain.id),
        });

Since these are coming from the same getEvents called and extracted out.

I"ve done several refreshes and haven't seen any weird behavior so far. But I guess thats no saying much.

Da-Colon commented 13 hours ago

In the latest commit, is there a race condition here? Seems like this code will only work correctly if and only if the KeyValuePairs data comes in (and gets stored in the map) before the updateRolesWithStreams function runs. Is that a valid concern?

I don't think so? Since its coming in is being fetched at the same time as the hatTreeId which kicks off the retreival of that hats and setHatsTree is first called. I believe that these will correctly get set prior to setHatTree being called next.

I thought about update setHatTreeId into like setHatTreeEventData (The name is why I didn't) to set the loop first. In that I effect. I could just flip These like this for the same effect.
   setHatIdsToStreamIds(getHatIdsToStreamIds(safeEvents, sablierV2LockupLinear, chain.id));
        setHatsTreeId({
          contextChainId: chain.id,
          hatsTreeId: getHatsTreeId(safeEvents, chain.id),
        });
Since these are coming from the same getEvents called and extracted out.

I"ve done several refreshes and haven't seen any weird behavior so far. But I guess thats no saying much.

@DarksightKellar @mudrila Do either of you have any thoughts on this?

decentdao / decent-interface