Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/distribution details #156

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Seroxdesign
Copy link
Contributor

PR:

Addresses: #19

What was done?

Created distribution page with the following:

Addresses, amount and pool percentage
image

Changes made:

export const getDistributionDetails = (distributionId: string) => `{
  indexUpdatedEvent(id: "${distributionId}") {
    index {
      indexId
      indexValue
      totalAmountDistributedUntilUpdatedAt
      totalSubscriptionsWithUnits
      totalUnits
      subscriptions(where: {units_not: "0"}) {
        units
        totalAmountReceivedUntilUpdatedAt
        subscriber {
          id
        }
      }
      token {
        id
      }
    }
    timestamp
    publisher
    totalUnitsApproved
    totalUnitsPending
    newIndexValue
    oldIndexValue
  }
}`;

Event showcase:

2023-02-27.06-22-28.mp4

Closes: #149

@vercel
Copy link

vercel bot commented Feb 27, 2023

@Seroxdesign is attempting to deploy a commit to the Superfluid Finance Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Feb 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
superfluid-console ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 27, 2023 at 0:14AM (UTC)

@kasparkallas
Copy link
Collaborator

Hey! I'll take a look at this. Any pending questions or TODO-s?

@kasparkallas kasparkallas self-requested a review February 27, 2023 11:32
@Seroxdesign Seroxdesign changed the title Feature/distribution details Feature/distribution details - Ready for review Feb 27, 2023
@Seroxdesign
Copy link
Contributor Author

Seroxdesign commented Feb 27, 2023

Hey! I'll take a look at this. Any pending questions or TODO-s?

Yes, I am quering the SF subgraph as follows:

 useEffect(() => {
    if (!distributionId) return;
    const fetchData = async () => {
      await fetch(baseUrl, {
        method: "POST",
        headers: new Headers({
          "Content-Type": "application/json",
        }),
        body: JSON.stringify({
          query: `${getDistributionDetails(distributionId)}`,
        }),
      })
        .then((res) => res.json())
        .then((res) => {
          const distribution = res.data.indexUpdatedEvent;
          if (distribution) {
            handleSetDistributionDetails(distribution);
          }
        });
    };
    fetchData();
  }, [distributionId]);

Unfortunately I couldn't find docs or examples on how to use sfSubgraph directly.

  • I forgot to add an export, just pushed a fix so deploy should work now, can you re-authorise? @kasparkallas

@elvijsTDL
Copy link
Contributor

@Seroxdesign I come once again to ask for bug fixes 😄

Check this one out: https://superfluid-console-git-fork-hunters-w-7430a0-superfluid-finance.vercel.app/matic/distributions/IndexUpdated-0xd572b6c175fc462f48c2885d13ada0cd8dc7cb1a1fa7c0c0d11c0ede98ca2658-611

The total amount does not add up to the subscribed amount
Screenshot 2023-02-28 at 11 52 56

Also it seems like the feature does not show the total units at the point of distribution and the subscribers correctly , that looks like a stream to ricochet and if i'm not mistaken, the units there are updated depending on the flow rate, maybe just a coinsidence, but seems very unlikely that the units and subscribers have not changed in months , especially seeing that the total amount is wrong, I feel like it fetches 1 state and shows it in all distribution details pages

Screen.Recording.2023-02-28.at.12.01.06.mov

@Seroxdesign
Copy link
Contributor Author

@Seroxdesign I come once again to ask for bug fixes 😄

Check this one out: https://superfluid-console-git-fork-hunters-w-7430a0-superfluid-finance.vercel.app/matic/distributions/IndexUpdated-0xd572b6c175fc462f48c2885d13ada0cd8dc7cb1a1fa7c0c0d11c0ede98ca2658-611

The total amount does not add up to the subscribed amount

Screenshot 2023-02-28 at 11 52 56

Also it seems like the feature does not show the total units at the point of distribution and the subscribers correctly , that looks like a stream to ricochet and if i'm not mistaken, the units there are updated depending on the flow rate, maybe just a coinsidence, but seems very unlikely that the units and subscribers have not changed in months , especially seeing that the total amount is wrong, I feel like it fetches 1 state and shows it in all distribution details pages

Screen.Recording.2023-02-28.at.12.01.06.mov

Understood will investigate this further and fix it thanks for taking the time to test and review 🙏

totalSubscriptionsWithUnits
totalUnits
subscriptions(where: {units_not: "0"}) {
units
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These units might not be the same as they were at the time of the distribution. These are the latest units.

@kasparkallas
Copy link
Collaborator

kasparkallas commented Feb 28, 2023

Understood will investigate this further and fix it thanks for taking the time to test and review

Be careful, might be a rabbit hole.

Sorry, I haven't been quick to review this. It's a bit more tricky... I skimmed over it yesterday and it made my wheels turn.

I'm thinking open-minded and it might be the case that the best way to solve this page is:

  1. Get the distribution event (with the block number it happened at)
  2. Query the subscriptions for that index at that block number.
  3. Loop and match in the UI

Example of the query at 2:

query MyQuery {
  indexSubscriptions(block: {number: $BLOCK_NUMBER}, where: {index: $INDEX_ID}) {
    units
  }
}
  • The SDK supports querying by block number.
  • The Subgraph has history of every entity for every block number. It's like that to handle re-orgs and time-travel queries like this.

@Seroxdesign Seroxdesign changed the title Feature/distribution details - Ready for review Feature/distribution details Mar 8, 2023
@Seroxdesign
Copy link
Contributor Author

As per @kasparkallas the bounty can be marked as complete, there was an issue I couldn't address and Kaspar will be taking a look at it, if anything comes up that i can help with ping me.

@vmichalik vmichalik requested a review from tokdaniel March 20, 2023 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants