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

Yana/pending staking events #5400

Merged
merged 86 commits into from
Feb 12, 2025
Merged

Conversation

yanok87
Copy link
Contributor

@yanok87 yanok87 commented Jan 29, 2025

This change is Reviewable

@yanok87 yanok87 requested a review from fmtabbara January 29, 2025 18:50
Copy link

vercel bot commented Jan 29, 2025

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

Name Status Preview Comments Updated (UTC)
nym-next-explorer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 12, 2025 0:43am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
docs-nextra ⬜️ Ignored (Inspect) Visit Preview Feb 12, 2025 0:43am

Comment on lines +5 to 10
import { AccountBalancesCard } from "../../../../components/accountPageComponents/AccountBalancesCard";
import { AccountInfoCard } from "../../../../components/accountPageComponents/AccountInfoCard";
import { ContentLayout } from "../../../../components/contentLayout/ContentLayout";
import SectionHeading from "../../../../components/headings/SectionHeading";
import ExplorerButtonGroup from "../../../../components/toggleButton/ToggleButton";

Copy link
Contributor

Choose a reason for hiding this comment

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

can you use the @ import alias for imports. It's a bit neater.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for some reason I've started having import red errors with "@/...", have refactored to this to get rid of the errors

Copy link
Contributor

Choose a reason for hiding this comment

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

what are the errors?

Comment on lines 27 to 64
const [hasEpochStarted, setHasEpochStarted] = useState(false);
const [loading, setLoading] = useState<string | null>(null);

const { data } = useQuery({
enabled: true,
queryKey: ["currentEpoch"],
queryFn: fetchCurrentEpoch,
refetchInterval: 30000,
staleTime: 30000,
refetchOnMount: true,
keepPreviousData: false,
});

const queryClient = useQueryClient();
// check epoch update
useEffect(() => {
const checkEpochStatus = () => {
if (!data?.dateTime) return;
const oneHourLater = subSeconds(new Date(data.dateTime), 30).getTime();
const now = Date.now();
setHasEpochStarted(now >= oneHourLater);
};

checkEpochStatus();
const interval = setInterval(checkEpochStatus, 60000);
return () => clearInterval(interval);
});

const handleRefetch = useCallback(() => {
queryClient.invalidateQueries();
}, [queryClient]);

useEffect(() => {
if (!hasEpochStarted) return;
handleRefetch();
const interval = setInterval(handleRefetch, 30000);
return () => clearInterval(interval);
}, [hasEpochStarted, handleRefetch]);
Copy link
Contributor

Choose a reason for hiding this comment

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

this logic cant live in this component. what if want to use the component somewhere else in the app and have it do something different? It should have an onToggle prop or the logic should live on the page it needs to work on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, will find a better component to hang the epoch listening functionality on it

Comment on lines 43 to 53
const checkEpochStatus = () => {
if (!data?.dateTime) return;
const oneHourLater = subSeconds(new Date(data.dateTime), 30).getTime();
const now = Date.now();
setHasEpochStarted(now >= oneHourLater);
};

checkEpochStatus();
const interval = setInterval(checkEpochStatus, 60000);
return () => clearInterval(interval);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

can you explain this as I'm not fully clear on what is going on or why we need this?

Copy link
Contributor Author

@yanok87 yanok87 Feb 10, 2025

Choose a reason for hiding this comment

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

when the epoch time changes, there's a delay of the new data being returned from the fetch request, because there is a cache on the contract data. So, even if it's past the current epoch time, the fetch requests still return the last epoch time, resulting into bad UX. For example, it looks buggy if it's written next epoch starts at 18:26pm and it's already 18:29pm. I've tested and had up to 5min delay... It's always different, but always a delay.
Therefore, I was advised to show "Waiting for the next epoch to start.." until we get new data back.
Also, if the new epoch has started, I'm resetting all the queries and fetching new data across the app. Lot's of queries get updated even if the contract epoch hasn't returned new epoch time yet, because not all endpoints are updated simultaneously with the epoch change. Some update sooner, some later.

Copy link
Contributor

Choose a reason for hiding this comment

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

why don't we check the current time against most recently fetched epoch time, if the currenttime > epochtime show "getting new epoch time" (or something like that). If we check the epoch every 30 or 60 secs eventually it's going to show correctly?

The frontend shouldnt resolving issues in epoch timing. If there's an issue with caching on the BE it should be fixed there.

};

checkEpochStatus();
const interval = setInterval(checkEpochStatus, 60000);
Copy link
Contributor

Choose a reason for hiding this comment

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

if this block of logic is used in other parts of the app the it would be worth making a custom hook. But before that let's figure our what it's doing and why it's needed.


useEffect(() => {
const refreshQueries = async () => {
await QueryClient.invalidateQueries({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fmtabbara this fixes the infinite loop :)

@fmtabbara fmtabbara merged commit 9aed938 into feature/explorer_v2 Feb 12, 2025
6 checks passed
@fmtabbara fmtabbara deleted the yana/pending-staking-events branch February 12, 2025 13:15
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.

2 participants