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.

Sign up for GitHub

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

fix: debug multiple account request #580

Merged
merged 4 commits into from
Dec 13, 2024
Merged

fix: debug multiple account request #580

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3a30e36
fix: change all staking contract details loaded check to use some ins…
mohandast52 Dec 13, 2024
6503134
fix: optimize EachAgent component with memoization and improve code c…
mohandast52 Dec 13, 2024
f38dec1
fix: update comment for clarity and remove unnecessary console logs i…
mohandast52 Dec 13, 2024
a3391e9
fix: handle empty staking information with an alert message
mohandast52 Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 75 additions & 64 deletions frontend/components/AgentSelection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Button, Card, Flex, Typography } from 'antd';
import { entries } from 'lodash';
import Image from 'next/image';
import { useCallback } from 'react';
import { memo, useCallback } from 'react';

import { AGENT_CONFIG } from '@/config/agents';
import { COLOR } from '@/constants/colors';
Expand All @@ -13,6 +13,7 @@ import { useServices } from '@/hooks/useServices';
import { useSetup } from '@/hooks/useSetup';
import { useMasterWalletContext } from '@/hooks/useWallet';
import { AgentConfig } from '@/types/Agent';
import { delayInSeconds } from '@/utils/delay';

import { SetupCreateHeader } from './SetupPage/Create/SetupCreateHeader';
import { CardFlex } from './styled/CardFlex';
Expand All @@ -25,29 +26,37 @@ type EachAgentProps = {
agentConfig: AgentConfig;
};

const EachAgent = ({
showSelected,
agentType,
agentConfig,
}: EachAgentProps) => {
const { goto: gotoSetup } = useSetup();
const { goto: gotoPage } = usePageState();
const { selectedAgentType, updateAgentType } = useServices();
const { masterSafes, isLoading } = useMasterWalletContext();
const EachAgent = memo(
({ showSelected, agentType, agentConfig }: EachAgentProps) => {
const { goto: gotoSetup } = useSetup();
const { goto: gotoPage } = usePageState();
const { selectedAgentType, updateAgentType } = useServices();
const { masterSafes, isLoading } = useMasterWalletContext();

const isCurrentAgent = showSelected ? selectedAgentType === agentType : false;
const isCurrentAgent = showSelected
? selectedAgentType === agentType
: false;

const handleSelectAgent = useCallback(() => {
updateAgentType(agentType);
const handleSelectAgent = useCallback(async () => {
updateAgentType(agentType);

const isSafeCreated = masterSafes?.find(
(masterSafe) =>
masterSafe.evmChainId === AGENT_CONFIG[agentType].evmHomeChainId,
);
// DO NOTE REMOVE THIS DELAY
// NOTE: the delay is added so that agentType is updated in electron store
// and re-rendered with the updated agentType
await delayInSeconds(0.5);
Copy link
Collaborator

@Tanya-atatakai Tanya-atatakai Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

ahh interesting, love such hacks :D
but ideally would store a reference to the new value (or a state), update it inside the handleSelectAgent right after updating the store value, and then in a separate useEffect would check the value from store, compare it with the ref (or state) and call all the below

but this fine with me too, so approving

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was the same as what you are mentioning 🙈 , but adding a delay is a better than overcomplicating things with two states pointing to the same value and keeping them in sync. WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

it's not pointing to the same values, it's just the matter of synchronising them :D something similar to what we do on login when checking if the onboarding wasn't complete. just hope 0.5 sec delay is 100% stable


const isSafeCreated = masterSafes?.find(
(masterSafe) =>
masterSafe.evmChainId === AGENT_CONFIG[agentType].evmHomeChainId,
);

// If safe is created for the agent type, then go to main page
if (isSafeCreated) {
gotoPage(Pages.Main);
return;
}

if (isSafeCreated) {
gotoPage(Pages.Main);
} else {
// If safe is NOT created, then go to setup page based on the agent type
if (agentType === AgentType.Memeooorr) {
// if the selected type is Memeooorr - should set up the agent first
gotoPage(Pages.Setup);
Expand All @@ -56,53 +65,55 @@ const EachAgent = ({
gotoPage(Pages.Setup);
gotoSetup(SetupScreen.SetupEoaFunding);
}
}
}, [agentType, gotoPage, gotoSetup, masterSafes, updateAgentType]);

return (
<Card
key={agentType}
style={{ padding: 0, marginBottom: 6 }}
styles={{
body: {
padding: '12px 16px',
gap: 6,
borderRadius: 'inherit',
background: isCurrentAgent ? COLOR.GRAY_1 : 'transparent',
opacity: isCurrentAgent ? 0.75 : 1,
},
}}
>
<Flex vertical>
<Flex align="center" justify="space-between" className="mb-8">
<Image
src={`/agent-${agentType}-icon.png`}
width={36}
height={36}
alt={agentConfig.displayName}
/>
{isCurrentAgent ? (
<Text>Selected Agent</Text>
) : (
<Button
type="primary"
onClick={handleSelectAgent}
disabled={isLoading}
>
Select
</Button>
)}
}, [agentType, gotoPage, gotoSetup, masterSafes, updateAgentType]);

return (
<Card
key={agentType}
style={{ padding: 0, marginBottom: 6 }}
styles={{
body: {
padding: '12px 16px',
gap: 6,
borderRadius: 'inherit',
background: isCurrentAgent ? COLOR.GRAY_1 : 'transparent',
opacity: isCurrentAgent ? 0.75 : 1,
},
}}
>
<Flex vertical>
<Flex align="center" justify="space-between" className="mb-8">
<Image
src={`/agent-${agentType}-icon.png`}
width={36}
height={36}
alt={agentConfig.displayName}
/>
{isCurrentAgent ? (
<Text>Selected Agent</Text>
) : (
<Button
type="primary"
onClick={handleSelectAgent}
disabled={isLoading}
>
Select
</Button>
)}
</Flex>
</Flex>
</Flex>

<Title level={5} className="m-0">
{agentConfig.displayName}
</Title>
<Title level={5} className="m-0">
{agentConfig.displayName}
</Title>

<Text type="secondary">{agentConfig.description}</Text>
</Card>
);
};
<Text type="secondary">{agentConfig.description}</Text>
</Card>
);
},
);

EachAgent.displayName = 'EachAgent';

type AgentSelectionProps = {
showSelected?: boolean;
Expand Down
4 changes: 2 additions & 2 deletions frontend/components/SetupPage/Create/SetupEoaFunding.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ const EOA_FUNDING_MAP: Record<
// requiredEth: MIN_ETH_BALANCE_THRESHOLDS[EvmChainId.Base].safeCreation,
// },
// }
};
} as const;

/**
* EOA funding setup screen
Expand Down Expand Up @@ -221,8 +221,8 @@ export const SetupEoaFunding = () => {
const indexOfCurrentChain = chains.indexOf(currentChain.toString());
const nextChainExists = chains.length > indexOfCurrentChain + 1;

// goto next chain
if (nextChainExists) {
// goto next chain
setCurrentChain(chains[indexOfCurrentChain + 1] as unknown as EvmChainId);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion frontend/config/agents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,4 @@ export const AGENT_CONFIG: {
description:
'Autonomously post to Twitter, create and trade memecoins, and interact with other agents.',
},
};
} as const;
3 changes: 2 additions & 1 deletion frontend/context/StakingContractDetailsProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ const useAllStakingContractDetails = () => {
{} as Record<string, Partial<StakingContractDetails>>,
);

const isAllStakingContractDetailsLoaded = queryResults.every(
// TODO: some are failing, not sure why.
Copy link
Collaborator

@Tanya-atatakai Tanya-atatakai Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

can be due to forks/testnets

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, but "every" still doesn’t feel right. I think we should just remove the failed ones.

const isAllStakingContractDetailsLoaded = queryResults.some(
(query) => query.isSuccess,
);

Expand Down
Loading