Skip to content

Commit acd7421

Browse files
authored
fix(APP-462): Disable decoded view for raw calldata actions and fix native transfer action rendering (#642)
Signed-off-by: Milos Dzepina <milos@aragon.org>
1 parent 4f1b23f commit acd7421

File tree

8 files changed

+124
-8
lines changed

8 files changed

+124
-8
lines changed

.changeset/fruity-pandas-report.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@aragon/gov-ui-kit': patch
3+
---
4+
5+
Fix native transfer action decoding

.changeset/six-plants-lick.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@aragon/gov-ui-kit': patch
3+
---
4+
5+
Disable decoded view for raw calldata actions

pnpm-lock.yaml

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoder.test.tsx

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,4 +194,40 @@ describe('<ProposalActionsDecoder /> component', () => {
194194
);
195195
expect(() => render(createTestComponent({ action, view, mode }))).not.toThrow();
196196
});
197+
198+
it('displays no params message for write actions without parameters in decoded view', () => {
199+
const view = ProposalActionsDecoderView.DECODED;
200+
const mode = ProposalActionsDecoderMode.WATCH;
201+
const action = generateProposalAction({
202+
inputData: { function: 'withdraw', contract: '', parameters: [], stateMutability: 'nonpayable' },
203+
});
204+
render(createTestComponent({ action, view, mode }));
205+
expect(screen.getByText(modulesCopy.proposalActionsDecoder.noParametersMessage)).toBeInTheDocument();
206+
});
207+
208+
it('pre-populates data field with function selector for write actions without parameters in decoded edit view', () => {
209+
const view = ProposalActionsDecoderView.DECODED;
210+
const mode = ProposalActionsDecoderMode.EDIT;
211+
const action = generateProposalAction({
212+
data: '0x',
213+
inputData: { function: 'withdraw', contract: '', parameters: [], stateMutability: 'nonpayable' },
214+
});
215+
const setValueSpy = jest.fn();
216+
const getValuesSpy = jest.fn().mockReturnValue('0x');
217+
const watch = () => {
218+
return { unsubscribe: jest.fn() };
219+
};
220+
// Mock encodeFunctionData to return a function selector
221+
encodeFunctionDataSpy.mockReturnValue('0x12345678');
222+
useFormContextSpy.mockReturnValue(
223+
generateFormContext({
224+
setValue: setValueSpy,
225+
getValues: getValuesSpy,
226+
watch: watch as unknown as ModuleHooks.UseFormContextReturn['watch'],
227+
}),
228+
);
229+
render(createTestComponent({ action, view, mode }));
230+
// Verify that setValue was called to pre-populate the data field with the encoded function selector
231+
expect(setValueSpy).toHaveBeenCalledWith(expect.stringContaining('data'), '0x12345678');
232+
});
197233
});

src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoder.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ export const ProposalActionsDecoder: React.FC<IProposalActionsDecoderProps> = (p
118118
mode={mode}
119119
formPrefix={formPrefix}
120120
parameter={{ name: 'data', value: data, type: 'bytes' }}
121+
// Render the data field as hidden on decoded view to register the field on the form on EDIT mode
121122
className={view === ProposalActionsDecoderView.DECODED ? 'hidden' : undefined}
122123
component="textarea"
123124
/>

src/modules/components/proposal/proposalActions/proposalActionsDefinitions.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import type { ComponentType } from 'react';
22
import type { IWeb3ComponentProps } from '../../../types';
33

4+
/**
5+
* Proposal action types for which the basic view is available.
6+
*/
47
export enum ProposalActionType {
58
WITHDRAW_TOKEN = 'WITHDRAW_TOKEN',
69
ADD_MEMBERS = 'ADD_MEMBERS',
@@ -12,6 +15,14 @@ export enum ProposalActionType {
1215
UPDATE_PLUGIN_METADATA = 'UPDATE_PLUGIN_METADATA',
1316
}
1417

18+
/**
19+
* Proposal action types for which the basic view is NOT available.
20+
* This is not a list of all available types, but only the ones that are relevant in ui-kit context.
21+
*/
22+
export enum ProposalActionTypeNoBasicView {
23+
RAW_CALLDATA = 'RAW_CALLDATA',
24+
}
25+
1526
export interface IProposalActionInputDataParameter {
1627
/**
1728
* The name of the parameter being passed.

src/modules/components/proposal/proposalActions/proposalActionsItem/proposalActionsItem.test.tsx

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,4 +352,31 @@ describe('<ProposalActionsItem /> component', () => {
352352
await userEvent.click(screen.getByRole('menuitem', { name: modulesCopy.proposalActionsItem.menu.RAW }));
353353
expect(screen.getByTestId('decoder-mock').dataset.mode).toEqual(ProposalActionsDecoderMode.WATCH);
354354
});
355+
356+
it('renders the raw-view in edit mode when editMode prop is true and action is RAW_CALLDATA', async () => {
357+
const action = generateProposalAction({
358+
type: 'RAW_CALLDATA',
359+
inputData: { contract: '', function: '', parameters: [] },
360+
});
361+
isActionSupportedSpy.mockReturnValue(false);
362+
render(createTestComponent({ action, editMode: true }));
363+
await userEvent.click(screen.getByRole('button', { name: modulesCopy.proposalActionsItem.menu.dropdownLabel }));
364+
await userEvent.click(screen.getByRole('menuitem', { name: modulesCopy.proposalActionsItem.menu.RAW }));
365+
expect(screen.getByTestId('decoder-mock').dataset.view).toEqual(ProposalActionsDecoderView.RAW);
366+
expect(screen.getByTestId('decoder-mock').dataset.mode).toEqual(ProposalActionsDecoderMode.EDIT);
367+
expect(screen.queryByTestId('basic-view-mock')).not.toBeInTheDocument();
368+
});
369+
370+
it('renders the basic-view and raw view in watch mode when editMode prop is true and action is native transfer', async () => {
371+
const CustomComponent = () => 'custom';
372+
const action = generateProposalAction({ value: '1000000000000000000', data: '0x' });
373+
374+
render(createTestComponent({ action, CustomComponent, editMode: true }));
375+
376+
expect(screen.getByTestId('basic-view-mock')).toBeInTheDocument();
377+
await userEvent.click(screen.getByRole('button', { name: modulesCopy.proposalActionsItem.menu.dropdownLabel }));
378+
await userEvent.click(screen.getByRole('menuitem', { name: modulesCopy.proposalActionsItem.menu.RAW }));
379+
expect(screen.getByTestId('decoder-mock').dataset.view).toEqual(ProposalActionsDecoderView.RAW);
380+
expect(screen.getByTestId('decoder-mock').dataset.mode).toEqual(ProposalActionsDecoderMode.WATCH);
381+
});
355382
});

src/modules/components/proposal/proposalActions/proposalActionsItem/proposalActionsItem.tsx

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
import classNames from 'classnames';
2-
import { useEffect, useRef, useState } from 'react';
2+
import { useEffect, useMemo, useRef, useState } from 'react';
33
import { useFormContext, useWatch, type Control } from 'react-hook-form';
44
import { formatUnits } from 'viem';
55
import { mainnet } from 'viem/chains';
66
import { useChains } from 'wagmi';
77
import { Accordion, AlertCard, Button, Dropdown, IconType, Tooltip, invariant } from '../../../../../core';
88
import { useGukModulesContext } from '../../../gukModulesProvider';
9-
import { SmartContractFunctionDataListItem } from '../../../smartContract/smartContractFunctionDataListItem';
9+
import { SmartContractFunctionDataListItem } from '../../../smartContract';
1010
import { useProposalActionsContext } from '../proposalActionsContext';
11-
import { ProposalActionsDecoder, ProposalActionsDecoderView } from '../proposalActionsDecoder';
12-
import { ProposalActionsDecoderMode } from '../proposalActionsDecoder/proposalActionsDecoder.api';
13-
import type { IProposalAction } from '../proposalActionsDefinitions';
11+
import {
12+
ProposalActionsDecoder,
13+
ProposalActionsDecoderMode,
14+
ProposalActionsDecoderView,
15+
} from '../proposalActionsDecoder';
16+
import { ProposalActionTypeNoBasicView, type IProposalAction } from '../proposalActionsDefinitions';
1417
import type { IProposalActionsItemProps, ProposalActionsItemViewMode } from './proposalActionsItem.api';
1518
import { ProposalActionsItemBasicView } from './proposalActionsItemBasicView';
1619
import { proposalActionsItemUtils } from './proposalActionsItemUtils';
@@ -76,8 +79,19 @@ export const ProposalActionsItem = <TAction extends IProposalAction = IProposalA
7679
const shouldScrollRef = useRef(false);
7780
const supportsBasicView = CustomComponent != null || proposalActionsItemUtils.isActionSupported(action);
7881

82+
// View mode support depends on action type and ABI availability. Important cases:
83+
// - Actions with a basic view: decoded and raw views enabled in "watch" mode (no edits)
84+
// - Actions without a basic view (ABI is NOT present): decoded view disabled; raw view in edit mode
85+
// - Actions without a basic view (ABI is present): decoded view enabled in edit mode; raw view in watch mode
86+
// - Exception: write actions without params show the "no params" message in decoded view; raw view in watch mode
87+
// with pre-populated data field with fn selector, because leaving the data field empty was confusing and could
88+
// cause a failure when the selector is not added manually
89+
// - RAW_CALLDATA: no params case, but decoded view disabled; raw view enabled in edit mode for calldata input
90+
// - Native transfer: basic view available; decoded view disabled; raw view in watch mode
7991
const isAbiAvailable = action.inputData != null;
80-
const supportsDecodedView = isAbiAvailable;
92+
const isRawCalldataAction = action.type === (ProposalActionTypeNoBasicView.RAW_CALLDATA as string);
93+
const isNativeTransfer = action.data === '0x';
94+
const supportsDecodedView = isAbiAvailable && !isRawCalldataAction && !isNativeTransfer;
8195

8296
const [activeViewMode, setActiveViewMode] = useState<ProposalActionsItemViewMode>(
8397
supportsBasicView
@@ -152,8 +166,23 @@ export const ProposalActionsItem = <TAction extends IProposalAction = IProposalA
152166
];
153167

154168
const { EDIT, WATCH, READ } = ProposalActionsDecoderMode;
155-
const decodedViewMode = editMode && !supportsBasicView ? EDIT : editMode ? WATCH : READ;
156-
const rawViewMode = editMode && !supportsDecodedView ? EDIT : editMode ? WATCH : READ;
169+
170+
const decodedViewMode = useMemo(() => {
171+
if (!editMode) {
172+
return READ;
173+
}
174+
175+
return supportsBasicView ? WATCH : EDIT;
176+
}, [EDIT, READ, WATCH, editMode, supportsBasicView]);
177+
178+
const rawViewMode = useMemo(() => {
179+
if (!editMode) {
180+
return READ;
181+
}
182+
183+
// There is a case when basic view is supported but not decoded view, i.e., native transfer
184+
return supportsDecodedView || supportsBasicView ? WATCH : EDIT;
185+
}, [EDIT, READ, WATCH, editMode, supportsBasicView, supportsDecodedView]);
157186

158187
return (
159188
<Accordion.Item value={value ?? index.toString()} ref={itemRef}>

0 commit comments

Comments
 (0)