Skip to content

Commit a947816

Browse files
RogerLamTdggonzalez94claude
authored
chore(dao-ui): codex reviews (#182)
Co-authored-by: Gustavo Gonzalez <ggonzalez94@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5d03c64 commit a947816

9 files changed

Lines changed: 171 additions & 11 deletions

File tree

packages/ui/src/components/WalletContainer.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { PUB_CHAIN, PUB_CHAIN_NAME } from "@/constants";
2+
import { useWalletChainPolicy } from "@/context/WalletChainPolicy";
23
import { config } from "@/context/Web3Modal";
34
import { formatHexString } from "@/utils/evm";
5+
import { shouldForcePrimaryChainSwitch } from "@/utils/wallet-chain-policy";
46
import { MemberAvatar } from "@aragon/ods";
57
import { useWeb3Modal } from "@web3modal/wagmi/react";
68
import classNames from "classnames";
@@ -16,6 +18,7 @@ const WalletContainer = () => {
1618
const { open } = useWeb3Modal();
1719
const { address, isConnected, chainId } = useAccount();
1820
const { switchChain } = useSwitchChain();
21+
const { allowedSecondaryChainIds } = useWalletChainPolicy();
1922

2023
const { data: ensName } = useEnsName({
2124
config,
@@ -32,11 +35,10 @@ const WalletContainer = () => {
3235
});
3336

3437
useEffect(() => {
35-
if (!chainId) return;
36-
else if (chainId === PUB_CHAIN.id) return;
38+
if (!shouldForcePrimaryChainSwitch(chainId, PUB_CHAIN.id, allowedSecondaryChainIds)) return;
3739

3840
switchChain({ chainId: PUB_CHAIN.id });
39-
}, [chainId, switchChain]);
41+
}, [allowedSecondaryChainIds, chainId, switchChain]);
4042

4143
return (
4244
<button

packages/ui/src/components/l2Execution/ProposalL2Execution.tsx

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1+
import { useEffect } from "react";
12
import { AlertInline, Button, Spinner } from "@aragon/ods";
23
import { useWeb3Modal } from "@web3modal/wagmi/react";
34
import { type Hex } from "viem";
45
import { useAccount, useSwitchChain } from "wagmi";
56
import { PUB_TAIKO_BRIDGE_ADDRESS, TAIKO_L2_CHAIN_ID } from "@/constants";
7+
import { useWalletChainPolicy } from "@/context/WalletChainPolicy";
68
import { useL2AnchorSync } from "@/hooks/useL2AnchorSync";
79
import { useL2LegExecution } from "@/hooks/useL2LegExecution";
10+
import { shouldRenderL2ExecutionCard } from "@/utils/l2-execution";
811
import { type RawAction } from "@/utils/types";
912

1013
interface ProposalL2ExecutionProps {
@@ -36,6 +39,7 @@ export function ProposalL2Execution({
3639
const { isConnected, chain } = useAccount();
3740
const { open } = useWeb3Modal();
3841
const { switchChain } = useSwitchChain();
42+
const { setAllowedSecondaryChainIds } = useWalletChainPolicy();
3943

4044
const l1BlockNumber = executionBlockNumber ? BigInt(executionBlockNumber) : undefined;
4145
const l1TxHash = executorTxHash as Hex | undefined;
@@ -68,9 +72,22 @@ export function ProposalL2Execution({
6872
const detectedFromTx = !!message;
6973

7074
const hasL2Leg = detectedFromActions || detectedFromTx;
75+
// Allow Taiko L2 only while still determining L2 leg status or when a confirmed L2 leg exists.
76+
// Without this guard, executed proposals with no L2 leg keep the secondary chain allowed indefinitely.
77+
const shouldAllowTaikoL2 = executed && shouldCheckL2 && !isL2Confirmed && (!isSynced || isExtracting || hasL2Leg);
78+
79+
useEffect(() => {
80+
setAllowedSecondaryChainIds(shouldAllowTaikoL2 ? [TAIKO_L2_CHAIN_ID] : []);
81+
82+
return () => {
83+
setAllowedSecondaryChainIds([]);
84+
};
85+
}, [setAllowedSecondaryChainIds, shouldAllowTaikoL2]);
7186

7287
// Don't render if no L2 leg detected (and not still checking)
73-
if (!hasL2Leg && !isExtracting) return null;
88+
if (!shouldRenderL2ExecutionCard({ hasL2Leg, isExtracting, shouldCheckExecutedProposal: shouldCheckL2 })) {
89+
return null;
90+
}
7491

7592
// Pre-execution: show informational message
7693
if (!executed) {
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import React, { createContext, useContext, useMemo, useState } from "react";
2+
3+
export interface WalletChainPolicyContextProps {
4+
allowedSecondaryChainIds: number[];
5+
setAllowedSecondaryChainIds: (chainIds: number[]) => void;
6+
}
7+
8+
const WalletChainPolicyContext = createContext<WalletChainPolicyContextProps | undefined>(undefined);
9+
10+
export const WalletChainPolicyProvider: React.FC<{ children: React.ReactNode }> = ({ children }) => {
11+
const [allowedSecondaryChainIds, setAllowedSecondaryChainIds] = useState<number[]>([]);
12+
13+
const value = useMemo(
14+
() => ({ allowedSecondaryChainIds, setAllowedSecondaryChainIds }),
15+
[allowedSecondaryChainIds]
16+
);
17+
18+
return <WalletChainPolicyContext.Provider value={value}>{children}</WalletChainPolicyContext.Provider>;
19+
};
20+
21+
export const useWalletChainPolicy = () => {
22+
const context = useContext(WalletChainPolicyContext);
23+
24+
if (!context) {
25+
throw new Error("useWalletChainPolicy must be used inside the WalletChainPolicyProvider");
26+
}
27+
28+
return context;
29+
};

packages/ui/src/context/index.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { PersistQueryClientProvider } from "@tanstack/react-query-persist-client
1111
import { UseDerivedWalletProvider } from "../hooks/useDerivedWallet";
1212
import { OdsModulesProvider } from "@aragon/ods";
1313
import { customModulesCopy, odsCoreProviderValues } from "@/components/ods-customizations";
14+
import { WalletChainPolicyProvider } from "./WalletChainPolicy";
1415

1516
const queryClient = new QueryClient({
1617
defaultOptions: {
@@ -53,7 +54,9 @@ export function RootContextProvider({ children }: { children: ReactNode }) {
5354
>
5455
<PersistQueryClientProvider client={queryClient} persistOptions={{ persister }}>
5556
<AlertProvider>
56-
<UseDerivedWalletProvider>{children}</UseDerivedWalletProvider>
57+
<WalletChainPolicyProvider>
58+
<UseDerivedWalletProvider>{children}</UseDerivedWalletProvider>
59+
</WalletChainPolicyProvider>
5760
</AlertProvider>
5861
</PersistQueryClientProvider>
5962
</OdsModulesProvider>

packages/ui/src/hooks/useL2LegExecution.ts

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
TAIKO_L2_CHAIN_ID,
2424
} from "@/constants";
2525
import { AlertContextProps, useAlerts } from "@/context/Alerts";
26+
import { getConfirmedL2MessageOutcome } from "@/utils/l2-execution";
2627

2728
// Bridge message status: 0 = NEW, 1 = RETRIABLE, 2 = DONE, 3 = FAILED, 4 = PROVEN
2829
const MESSAGE_STATUS_DONE = 2;
@@ -104,7 +105,7 @@ export function useL2LegExecution(
104105
}
105106
setIsExtracting(false);
106107
})
107-
.catch((err) => {
108+
.catch(() => {
108109
if (cancelled) return;
109110
setExtractError("Failed to read L1 transaction");
110111
setIsExtracting(false);
@@ -246,19 +247,39 @@ export function useL2LegExecution(
246247
return;
247248
}
248249
if (isL2Confirmed) {
249-
if (isMessageDone) {
250+
const messageOutcome = getConfirmedL2MessageOutcome(messageStatusResult);
251+
252+
if (messageOutcome === "success") {
250253
addAlert("L2 leg executed successfully", {
251254
description: "The cross-chain proposal actions have been executed on L2",
252255
type: "success",
253256
});
254-
} else {
255-
addAlert("L2 transaction confirmed but message not yet processed", {
256-
description: "The inner L2 execution may have failed. Check the transaction and retry if needed.",
257+
return;
258+
}
259+
260+
if (messageOutcome === "failed") {
261+
addAlert("L2 transaction confirmed but message processing failed", {
262+
description: "The bridge message failed on L2. Check the transaction and retry if needed.",
257263
type: "error",
258264
});
265+
return;
259266
}
267+
268+
addAlert("L2 transaction confirmed; waiting for message processing", {
269+
description: "The transaction was confirmed on Taiko L2 and the bridge message is still being finalized.",
270+
type: "info",
271+
});
260272
}
261-
}, [writeStatus, writeError, l2TxHash, isL2Confirming, isL2Confirmed, isMessageDone, addAlert, resetWrite]);
273+
}, [
274+
writeStatus,
275+
writeError,
276+
l2TxHash,
277+
isL2Confirming,
278+
isL2Confirmed,
279+
messageStatusResult,
280+
addAlert,
281+
resetWrite,
282+
]);
262283

263284
return {
264285
message,
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { describe, expect, test } from "bun:test";
2+
import {
3+
getConfirmedL2MessageOutcome,
4+
shouldRenderL2ExecutionCard,
5+
} from "../utils/l2-execution";
6+
7+
describe("shouldRenderL2ExecutionCard", () => {
8+
test("keeps the card visible while an executed proposal with a tx hash is still being checked", () => {
9+
expect(
10+
shouldRenderL2ExecutionCard({
11+
hasL2Leg: false,
12+
isExtracting: false,
13+
shouldCheckExecutedProposal: true,
14+
})
15+
).toBe(true);
16+
});
17+
18+
test("hides the card when there is no detected L2 leg and no active check", () => {
19+
expect(
20+
shouldRenderL2ExecutionCard({
21+
hasL2Leg: false,
22+
isExtracting: false,
23+
shouldCheckExecutedProposal: false,
24+
})
25+
).toBe(false);
26+
});
27+
});
28+
29+
describe("getConfirmedL2MessageOutcome", () => {
30+
test("treats DONE as a success", () => {
31+
expect(getConfirmedL2MessageOutcome(2)).toBe("success");
32+
});
33+
34+
test("treats FAILED as a failure", () => {
35+
expect(getConfirmedL2MessageOutcome(3)).toBe("failed");
36+
});
37+
38+
test("treats all other statuses as pending bridge processing", () => {
39+
expect(getConfirmedL2MessageOutcome(0)).toBe("pending");
40+
expect(getConfirmedL2MessageOutcome(1)).toBe("pending");
41+
expect(getConfirmedL2MessageOutcome(4)).toBe("pending");
42+
expect(getConfirmedL2MessageOutcome(undefined)).toBe("pending");
43+
});
44+
});
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { shouldForcePrimaryChainSwitch } from "../utils/wallet-chain-policy";
3+
4+
describe("shouldForcePrimaryChainSwitch", () => {
5+
test("force-switches when connected to Taiko but the current flow has not allowed it", () => {
6+
expect(shouldForcePrimaryChainSwitch(167000, 1, [])).toBe(true);
7+
});
8+
9+
test("does not force-switch when the current flow explicitly allows Taiko", () => {
10+
expect(shouldForcePrimaryChainSwitch(167000, 1, [167000])).toBe(false);
11+
});
12+
13+
test("does not force-switch when already on the primary chain", () => {
14+
expect(shouldForcePrimaryChainSwitch(1, 1, [])).toBe(false);
15+
});
16+
});
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
export function shouldRenderL2ExecutionCard({
2+
hasL2Leg,
3+
isExtracting,
4+
shouldCheckExecutedProposal,
5+
}: {
6+
hasL2Leg: boolean;
7+
isExtracting: boolean;
8+
shouldCheckExecutedProposal: boolean;
9+
}) {
10+
return hasL2Leg || isExtracting || shouldCheckExecutedProposal;
11+
}
12+
13+
export function getConfirmedL2MessageOutcome(messageStatus?: number) {
14+
if (messageStatus === 2) return "success";
15+
if (messageStatus === 3) return "failed";
16+
17+
return "pending";
18+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
export function shouldForcePrimaryChainSwitch(
2+
currentChainId: number | undefined,
3+
primaryChainId: number,
4+
allowedSecondaryChainIds: number[] = []
5+
) {
6+
if (!currentChainId) return false;
7+
if (currentChainId === primaryChainId) return false;
8+
9+
return !allowedSecondaryChainIds.includes(currentChainId);
10+
}

0 commit comments

Comments
 (0)