-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix balance related blockers for migration #612
Conversation
const displayedBalance = useMemo(() => { | ||
// olas across master wallets, safes and eoa | ||
// olas across master wallet -- safes and eoa -- on relevant chains for agent | ||
const masterWalletOlasBalance = masterWalletBalances?.reduce( | ||
(acc, { symbol, balance }) => { | ||
if (symbol === TokenSymbol.OLAS) { | ||
(acc, { symbol, balance, evmChainId }) => { | ||
if ( | ||
symbol === TokenSymbol.OLAS && | ||
selectedAgentConfig.requiresAgentSafesOn.includes(evmChainId) | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine to aggregate olas across potential chains for display balance
// olas across all wallets owned by selected service | ||
const serviceWalletOlasBalance = serviceWalletBalances?.reduce( | ||
(acc, { symbol, balance }) => { | ||
if (symbol === TokenSymbol.OLAS) { | ||
(acc, { symbol, balance, evmChainId }) => { | ||
if ( | ||
symbol === TokenSymbol.OLAS && | ||
selectedAgentConfig.requiresAgentSafesOn.includes(evmChainId) | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ensuring we don't calculate funds not on chains irrelevant to the agent
const requiredOlasDeposit = | ||
requiredStakedOlas - | ||
sum([ | ||
masterSafeBalanceOnHomeChain[TokenSymbol.OLAS], | ||
totalStakedOlasBalanceOnHomeChain, | ||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed this condition, may be breaking due to suspension or eviction
sum
allows for null/undefined so don't need to guard here
middleware will migrate funds from the current contract if migration required
const masterSafeOlasBalance = masterWalletBalances | ||
?.filter((walletBalance) => walletBalance.symbol === TokenSymbol.OLAS) | ||
?.filter( | ||
(walletBalance) => | ||
walletBalance.symbol === TokenSymbol.OLAS && | ||
selectedAgentConfig.requiresMasterSafesOn.includes( | ||
walletBalance.evmChainId, | ||
), | ||
) | ||
.reduce((acc, balance) => acc + balance.balance, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
found this confusing as funds from base where shown here while on predict
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, fixed in a different PR, please feel free to keep your change in the conflicts.
const masterSafeOlasBalance = masterWalletBalances | ||
?.filter((walletBalance) => walletBalance.symbol === TokenSymbol.OLAS) | ||
?.filter( | ||
(walletBalance) => | ||
walletBalance.symbol === TokenSymbol.OLAS && | ||
selectedAgentConfig.requiresMasterSafesOn.includes( | ||
walletBalance.evmChainId, | ||
), | ||
) | ||
.reduce((acc, balance) => acc + balance.balance, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, fixed in a different PR, please feel free to keep your change in the conflicts.
Proposed changes
Types of changes
What types of changes does your code introduce?
Put an
x
in the boxes that apply