feat: Batch txn UI #1242
feat: Batch txn UI #1242
Conversation
marshall2112
added
the
preview-deploy
|
Deploy preview for core ready! ✅ Preview Built with commit 95326b5. |
📝 WalkthroughWalkthroughAdds a BatchAuctionForm UI (two-tab: configure vs fund) and registers it in the Safe admin page behind new feature flags. Introduces env.spiceAuctionAdmin config and feature flags across env files. Exposes a proposeTransaction API through SafeTransactionsContext and implements it in useSafeSdk to support single-call and multisend batching, signing, and submission to the Safe V1 API. Exports two multisend encoder utilities (encodeMultiSendData, encodeMetaTransaction). Also adds an isDisabled prop and disabled-aware styles to an InputSelect component. Sequence Diagram(s)sequenceDiagram
actor User
participant BatchAuctionForm
participant SafeTransactionsContext
participant useSafeSdk
participant SafeSDK as Safe SDK
participant V1Service as Safe V1 API
User->>BatchAuctionForm: Fill form / click "Create Batch Transaction"
BatchAuctionForm->>BatchAuctionForm: Validate inputs & build txList
BatchAuctionForm->>SafeTransactionsContext: proposeTransaction(txList, targetSafe)
SafeTransactionsContext->>useSafeSdk: proposeTransaction(txList, targetSafe)
alt single transaction
useSafeSdk->>SafeSDK: build SafeTransaction (to, value, data, op)
else multiple transactions
useSafeSdk->>useSafeSdk: encodeMultiSendData(txList)
useSafeSdk->>SafeSDK: build MultiSend delegateCall payload
end
useSafeSdk->>SafeSDK: compute tx hash & sign
useSafeSdk->>V1Service: v1SafesMultisigTransactionsCreate(SafeMultisigTransaction)
V1Service-->>useSafeSdk: ack
useSafeSdk-->>SafeTransactionsContext: resolve
SafeTransactionsContext-->>BatchAuctionForm: success
BatchAuctionForm-->>User: display confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @apps/dapp/src/safe/sdk/use-safe-sdk.tsx:
- Around line 269-287: The unsafe "as any" on safeMultisigTx.value (currently
assigned from transactionData.value) masks a real type mismatch against
SafeMultisigTransaction; remove the cast and fix the type/serialization: either
update the SafeMultisigTransaction type to allow string values (string | number)
and ensure the HTTP client sends the string as the integer representation
expected by the Safe Transaction Service, or explicitly convert
transactionData.value to a numeric type (Number or BigInt → number/string)
before assignment if you accept possible precision loss; locate the
safeMultisigTx construction and change the value assignment accordingly (and
update related types or serialization logic so TypeScript verifies correctness).
🧹 Nitpick comments (6)
apps/dapp/src/components/InputSelect/InputSelect.tsx (1)
20-20: LGTM! Disabled state implementation is solid.The addition of the
isDisabledprop and its integration across the control, singleValue, and dropdownIndicator styling is well-executed. The visual feedback (cursor changes, opacity adjustments, and color variations) clearly communicates the disabled state to users.Since the props are spread to the underlying
react-selectcomponent (line 36), accessibility attributes should be handled automatically by the library.Optional: Consider nullish coalescing for the background color fallback
At line 69, the fallback pattern uses
||which could be replaced with??(nullish coalescing) for more precise behavior:- backgroundColor: state.isDisabled - ? theme.palette.brand25 || `${theme.palette.brand}40` - : theme.palette.dark, + backgroundColor: state.isDisabled + ? theme.palette.brand25 ?? `${theme.palette.brand}40` + : theme.palette.dark,This ensures the fallback only applies when
brand25isnullorundefined, not when it's other falsy values like0or"". For color values this is unlikely to matter in practice, but??better expresses the intent of providing a default for missing values.Also applies to: 56-72, 104-113, 118-128
apps/dapp/src/constants/env/local.tsx (1)
158-164: Consider validating admin addresses before use.The feature flags are enabled with placeholder
'0x0'addresses for local development. Ensure that code consumingspiceAuctionAdminaddresses validates them before use to prevent unexpected behavior during local development.💡 Optional: Add inline documentation for local setup
Consider adding a comment to guide developers on how to configure proper test addresses:
}, featureFlags: { enableAscend: true, enableGnosisSafeAdmin: true, enableSpiceAuctionAdmin: true, }, spiceAuctionAdmin: { + // TODO: Replace with actual test Safe address for local development multisigAddress: '0x0', + // TODO: Replace with actual test strategy address for local development cosechaSegundaAddress: '0x0', },apps/dapp/src/safe/safeContext.tsx (1)
24-27: UseMetaTransactionData[]instead ofany[]for type safety.The actual implementation in
use-safe-sdk.tsxusesMetaTransactionData[]. Usingany[]here loses type safety at the context boundary.🔎 Proposed fix
+import { MetaTransactionData } from '@safe-global/safe-core-sdk-types'; + interface ISafeTransactionsContext { safeAddress: string; isLoading: () => boolean; tableRows: ( safeTxCategory: SafeTransactionCategory, updateSafeTableRow: (safeTxHash: string, newValue?: SafeTableRow) => void ) => Promise<SafeTableRow[]>; proposeTransaction?: ( - transactions: any[], + transactions: MetaTransactionData[], targetSafeAddress?: string ) => Promise<void>; }apps/dapp/src/components/Pages/Safe/admin/BatchAuctionForm.tsx (3)
62-63: Unused state variableisTempleGoldAuctionToken.This state is declared but never used. On line 107,
isTempleGoldAuctionTokenis hardcoded asfalseinstead of using this state value.Either remove the unused state or wire it up to the form:
🔎 Option 1: Remove unused state
- const [isTempleGoldAuctionToken, setIsTempleGoldAuctionToken] = - useState<boolean>(false);🔎 Option 2: Use the state value
{ duration: Number(duration), waitPeriod: Number(waitPeriod), minimumDistributedAuctionToken: parseUnits( minimumDistributedAmount, 18 ).toString(), - isTempleGoldAuctionToken: false, + isTempleGoldAuctionToken: isTempleGoldAuctionToken, recipient: recipient, },
70-78: Misleading error message whenproposeTransactionis undefined.The error message says "Please connect your wallet" but the actual check is for
proposeTransactionexistence. This function could be undefined for reasons other than wallet connection (e.g., context not properly initialized).🔎 Proposed fix
if (!proposeTransaction) { openNotification({ - title: 'Please connect your wallet', + title: 'Transaction proposal unavailable', hash: '', isError: true, }); return; }
397-403: Remove unused styled components.The following styled components are defined but never used in the JSX:
Title,CheckboxContainer,Checkbox,CheckboxLabel,HelperText.🔎 Remove unused components
-const Title = styled.h3` - color: ${({ theme }) => theme.palette.brand}; - font-size: 1.1rem; - margin: 0; - padding-bottom: 0.5rem; - border-bottom: 1px solid ${({ theme }) => theme.palette.brand}; -`; - ... -const CheckboxContainer = styled.div` - display: flex; - align-items: center; - gap: 0.5rem; -`; - -const Checkbox = styled.input` - width: 18px; - height: 18px; - cursor: pointer; -`; - -const CheckboxLabel = styled.label` - color: ${({ theme }) => theme.palette.brandLight}; - font-size: 0.9rem; -`; - ... -const HelperText = styled.span` - color: ${({ theme }) => theme.palette.brandDark}; - font-size: 0.75rem; - font-style: italic; - margin-top: 0.25rem; -`;Also applies to: 478-493, 558-563
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/dapp/src/components/InputSelect/InputSelect.tsxapps/dapp/src/components/Pages/Safe/admin/BatchAuctionForm.tsxapps/dapp/src/components/Pages/Safe/admin/index.tsxapps/dapp/src/constants/env/local.tsxapps/dapp/src/constants/env/preview.tsxapps/dapp/src/constants/env/production.tsxapps/dapp/src/constants/env/types.tsapps/dapp/src/safe/safeContext.tsxapps/dapp/src/safe/sdk/use-safe-sdk.tsxapps/dapp/src/safe/sdk/utils/utils.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-30T23:20:17.523Z
Learnt from: frontier159
Repo: TempleDAO/temple PR: 1214
File: protocol/scripts/deploys/mainnet/team-payments/deploy.ts:9-9
Timestamp: 2025-08-30T23:20:17.523Z
Learning: In Temple DAO's protocol/scripts/deploys/mainnet/team-payments/deploy.ts, the user prefers to keep hard-coded imports for epoch snapshot JSON files (like epoch29b.json) rather than parametrizing them. Avoid suggesting refactoring to make snapshot files runtime parameters.
Applied to files:
apps/dapp/src/constants/env/preview.tsx
📚 Learning: 2025-11-20T01:06:33.012Z
Learnt from: marshall2112
Repo: TempleDAO/temple PR: 1224
File: apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/MyActivity/BidsForSpice/DataTables/BidDataTableMobile.tsx:238-243
Timestamp: 2025-11-20T01:06:33.012Z
Learning: In this Temple DAO codebase, the Button component (apps/dapp/src/components/Button/Button.tsx) has built-in async operation handling. It manages loading state internally, prevents double-clicks, awaits async onClick handlers, handles errors with try/catch, and displays a Loader component during operations. Components using styled(Button) like TradeButton inherit this functionality, so additional loading state management is not needed when onClick handlers return Promises.
Applied to files:
apps/dapp/src/components/Pages/Safe/admin/BatchAuctionForm.tsx
🧬 Code graph analysis (1)
apps/dapp/src/components/Pages/Safe/admin/index.tsx (4)
apps/dapp/src/components/Pages/Safe/admin/BatchAuctionForm.tsx (1)
BatchAuctionForm(35-389)apps/dapp/src/components/InputSelect/InputSelect.tsx (1)
InputSelect(32-132)apps/dapp/src/components/Copy/Copy.tsx (1)
Copy(8-19)apps/dapp/src/components/Pages/Safe/admin/SafeTxDataTable.tsx (1)
SafeTxsDataTable(40-174)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: vercel-deploy
- GitHub Check: Dapp Build
🔇 Additional comments (15)
apps/dapp/src/safe/sdk/utils/utils.ts (1)
292-303: LGTM: Clean export of encoding utilities.The export of
encodeMultiSendDataandencodeMetaTransactionis appropriate for enabling batch transaction construction outside this module. These are pure utility functions with no side effects, and exposing them supports the broader Safe transaction proposal functionality introduced in this PR.apps/dapp/src/constants/env/types.ts (1)
143-149: LGTM: Well-structured type additions for auction admin configuration.The new feature flags and
spiceAuctionAdminconfiguration object are cleanly integrated into theEnvironmentinterface. The structure clearly documents the required multisig and strategy addresses needed for the Spice Auction Admin feature.apps/dapp/src/constants/env/production.tsx (2)
361-363: Feature flags appropriately disabled in production.Both
enableGnosisSafeAdminandenableSpiceAuctionAdminare disabled in production, which is a safe default for a new feature. Ensure there's a plan and testing checklist in place before enabling these flags in production.
364-367: Verify the spiceAuctionAdmin addresses for correctness.The
cosechaSegundaAddressinspiceAuctionAdmin(0xA1f01d98d60273ED562eE79fEb718bbdB85E1d9C) differs fromstrategies.cosechaSegundaStrategyon line 253 (0x0cB92fEfae1806ed3047E3EB8F36bC963AC48637). Both are valid deployed contracts on mainnet but with different implementations. Confirm this is intentional and document the distinction between these addresses if they serve different purposes.apps/dapp/src/constants/env/preview.tsx (2)
173-179: Well-documented testnet configuration for preview environment.The Spice Auction Admin is appropriately enabled in preview with clearly documented Sepolia testnet addresses. The inline comments identifying the Safe multisigs are helpful for maintainability.
172-172: Unable to verify enableAscend flag change — file not found in repository.The file
apps/dapp/src/constants/env/preview.tsxand references to theenableAscendflag could not be located in the repository. This could indicate:
- The file path may be incorrect
- The change may have been reverted
- The PR context is not available in the current environment
Please confirm the exact file path and verify locally that this change is intentional by cross-referencing against:
- Related feature flag documentation or configuration
- Other environment configuration files (development, staging, production)
- Associated feature branch commits that introduced the change
apps/dapp/src/safe/safeContext.tsx (1)
44-47: LGTM!The
proposeTransactionfunction is correctly destructured fromuseSafeSdkand properly wired through the context provider.Also applies to: 213-213
apps/dapp/src/components/Pages/Safe/admin/index.tsx (2)
6-6: LGTM!Good defensive handling with optional chaining for
env.safes[0]?.address. The context provider is correctly set up to handle potentially empty safe addresses.Also applies to: 17-26
52-108: LGTM!Clean feature flag separation between Spice Auction Admin and Gnosis Safe Admin sections. Each feature can be independently enabled/disabled via environment configuration.
apps/dapp/src/safe/sdk/use-safe-sdk.tsx (3)
12-14: LGTM!The new imports are well-organized and support the multi-transaction proposal functionality.
Also applies to: 16-20, 26-28, 33-36, 38-38
176-239: LGTM!The
proposeTransactionimplementation correctly handles:
- Single transactions with
OperationType.Call- Multiple transactions via MultiSend with
OperationType.DelegateCall- Proper validation for empty transactions and missing addresses
241-266: LGTM!Transaction hash generation and signing flow is correctly implemented with proper V adjustment for
eth_sign.apps/dapp/src/components/Pages/Safe/admin/BatchAuctionForm.tsx (3)
91-169: LGTM!The transaction encoding logic correctly:
- Uses
SpiceAuction__factoryandERC20__factoryinterfaces for encoding- Handles config tab with single transaction
- Handles fund tab with batch (approve + fund) transactions
- Properly converts token amounts to wei with
parseUnitsBased on learnings, the Button component handles async operations internally, so no additional loading state management is needed.
368-389: LGTM!Clean tab-based UI structure with proper state management for active tab switching.
16-27: No action needed.
getAppConfig()returns a static configuration object determined at build time via Vite's environment variables (import.meta.env.VITE_ENV). These values are baked into the build and cannot change at runtime. ComputingAUCTION_OPTIONSat module load time is safe and appropriate; usinguseMemoor computing inside the component would be unnecessary.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @apps/dapp/src/safe/sdk/use-safe-sdk.tsx:
- Around line 268-296: The value field in the safeMultisigTx object is being
force-cast via transactionData.value as unknown as number which does not change
the runtime type and can violate the API input contract; replace this with a
proper conversion or type update: (1) confirm whether the Safe Transaction
Service accepts string inputs by testing the live API, (2) if the API requires a
numeric integer, convert transactionData.value to a numeric form (e.g., BigInt
-> number-safe serialization or integer) before assigning to value for
V1Service.v1SafesMultisigTransactionsCreate, or (3) if the API accepts strings,
update the TypeScript input type for SafeMultisigTransaction.value to string and
provide transactionData.value as a string to remove the unsafe cast; adjust
related type declarations and add validation/error handling around the
conversion.
🧹 Nitpick comments (1)
apps/dapp/src/safe/sdk/use-safe-sdk.tsx (1)
290-290: Consider parameterizing the origin string.The hardcoded
origin: 'Spice Auction Admin'identifies the transaction source. Consider making this a parameter or constant to improve maintainability if this method is reused for other admin contexts.Suggested refactor
const proposeTransaction = async ( transactions: MetaTransactionData[], - targetSafeAddress?: string + targetSafeAddress?: string, + origin?: string ): Promise<void> => { // ... existing code ... const safeMultisigTx: SafeMultisigTransaction = { // ... existing fields ... - origin: 'Spice Auction Admin', + origin: origin ?? 'Spice Auction Admin', };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/dapp/src/constants/env/preview.tsxapps/dapp/src/constants/env/production.tsxapps/dapp/src/safe/sdk/use-safe-sdk.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-30T23:20:17.523Z
Learnt from: frontier159
Repo: TempleDAO/temple PR: 1214
File: protocol/scripts/deploys/mainnet/team-payments/deploy.ts:9-9
Timestamp: 2025-08-30T23:20:17.523Z
Learning: In Temple DAO's protocol/scripts/deploys/mainnet/team-payments/deploy.ts, the user prefers to keep hard-coded imports for epoch snapshot JSON files (like epoch29b.json) rather than parametrizing them. Avoid suggesting refactoring to make snapshot files runtime parameters.
Applied to files:
apps/dapp/src/safe/sdk/use-safe-sdk.tsxapps/dapp/src/constants/env/production.tsx
🧬 Code graph analysis (1)
apps/dapp/src/safe/sdk/use-safe-sdk.tsx (4)
apps/dapp/src/safe/sdk/contracts/safeDeploymentContracts.ts (2)
getSafeContract(107-131)getMultiSendContract(133-157)apps/dapp/src/safe/sdk/contracts/config.ts (1)
DEFAULT_SAFE_VERSION(3-3)apps/dapp/src/safe/sdk/utils/utils.ts (4)
standardizeMetaTransactionData(358-366)encodeMultiSendData(292-294)ZERO_ADDRESS(31-31)getTransactionHash(209-230)apps/dapp/src/safe/sdk/signatures/utils.ts (1)
adjustVInSignature(54-99)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Dapp Build
- GitHub Check: vercel-deploy
🔇 Additional comments (4)
apps/dapp/src/safe/sdk/use-safe-sdk.tsx (3)
176-190: LGTM: Input validation and address handling.The method properly validates the signer presence and handles the optional
targetSafeAddressparameter with a sensible fallback to the context'ssafeAddress. The empty address check prevents API calls with invalid addresses.
209-239: LGTM: Transaction building logic handles single and batch cases correctly.The implementation properly distinguishes between single and multiple transactions:
- Single transaction: directly uses the transaction parameters
- Multiple transactions: correctly encodes via MultiSend with
DelegateCalloperationThe use of
standardizeMetaTransactionDataensures consistent defaults (e.g., operation type).
241-266: LGTM: Transaction hashing and signing implementation.The SafeTransaction is correctly constructed with zero gas parameters (appropriate for proposals), and the signing flow properly:
- Retrieves the transaction hash from the Safe contract
- Signs the hash using the signer
- Adjusts the signature's V value for
eth_signcompatibilityapps/dapp/src/constants/env/production.tsx (1)
361-367: Verify that the configured addresses match production mainnet requirements.The configuration is properly structured with consistent addresses across production and preview environments. However, ensure the following addresses are verified to be the correct production mainnet addresses for your deployment:
multisigAddress:0x94b62A27a2f23CBdc0220826a8452fB5055cF273cosechaSegundaAddress:0xA1f01d98d60273ED562eE79fEb718bbdB85E1d9CThese addresses are actively used in
BatchAuctionForm.tsxfor wallet configuration during auction administration. Confirm with your deployment documentation that these match the intended production contracts.
| spiceAuctionAdmin: { | ||
| multisigAddress: '0x94b62A27a2f23CBdc0220826a8452fB5055cF273', | ||
| cosechaSegundaAddress: '0xA1f01d98d60273ED562eE79fEb718bbdB85E1d9C', | ||
| }, |
There was a problem hiding this comment.
Address mismatch: Using mainnet addresses on Sepolia testnet.
The preview environment is configured for Sepolia testnet (network: 11155111, line 169), but the spiceAuctionAdmin addresses appear to be mainnet addresses that match production. This will likely cause runtime failures when the application attempts to interact with these contracts on Sepolia.
Verify whether:
- These contracts are deployed on Sepolia at the same addresses, or
- Different Sepolia-specific addresses should be used for the preview environment
Run the following script to check if these addresses exist on Sepolia:
#!/bin/bash
# Check if the addresses exist on Sepolia testnet
MULTISIG_ADDR="0x94b62A27a2f23CBdc0220826a8452fB5055cF273"
COSECHA_ADDR="0xA1f01d98d60273ED562eE79fEb718bbdB85E1d9C"
echo "=== Checking Multisig Address on Sepolia ==="
echo "Address: $MULTISIG_ADDR"
curl -s "https://api-sepolia.etherscan.io/api?module=proxy&action=eth_getCode&address=$MULTISIG_ADDR&tag=latest" | jq -r '.result'
echo ""
echo "=== Checking Cosecha Segunda Address on Sepolia ==="
echo "Address: $COSECHA_ADDR"
curl -s "https://api-sepolia.etherscan.io/api?module=proxy&action=eth_getCode&address=$COSECHA_ADDR&tag=latest" | jq -r '.result'
echo ""
# Check other strategy addresses for comparison
echo "=== Checking cosechaSegundaStrategy from contracts config ==="
rg "cosechaSegundaStrategy.*0x" apps/dapp/src/constants/env/preview.tsx| // Submit to Safe Transaction Service | ||
| const safeMultisigTx: SafeMultisigTransaction = { | ||
| safe: addressToUse, | ||
| to: transactionData.to, | ||
| // Type assertion required: OpenAPI spec incorrectly defines value as 'number', | ||
| // but we must use string to preserve precision for large wei amounts. | ||
| // Token amounts with 18 decimals produce 25-digit numbers that exceed | ||
| // JavaScript's safe integer range (2^53-1 = 16 digits). The Safe Transaction | ||
| // Service API accepts string values despite the spec (response types use string). | ||
| // The HTTP client serializes this as a JSON string, which the API handles correctly. | ||
| value: transactionData.value as unknown as number, | ||
| data: transactionData.data, | ||
| operation: transactionData.operation, | ||
| gasToken: ZERO_ADDRESS, | ||
| safeTxGas: 0, | ||
| baseGas: 0, | ||
| gasPrice: 0, | ||
| refundReceiver: ZERO_ADDRESS, | ||
| nonce, | ||
| contractTransactionHash: txHash, | ||
| sender: signerAddress, | ||
| signature, | ||
| origin: 'Spice Auction Admin', | ||
| }; | ||
|
|
||
| await V1Service.v1SafesMultisigTransactionsCreate( | ||
| addressToUse, | ||
| safeMultisigTx | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "*.json" -path "*/safe/*" -o -name "*api*" -o -name "*openapi*" | head -20Repository: TempleDAO/temple
Length of output: 176
🏁 Script executed:
rg "SafeMultisigTransaction" --type ts --type tsx -B 2 -A 8 | head -100Repository: TempleDAO/temple
Length of output: 176
🏁 Script executed:
rg "v1SafesMultisigTransactionsCreate" --type ts --type tsx -B 3 -A 3Repository: TempleDAO/temple
Length of output: 87
🏁 Script executed:
rg "interface SafeMultisigTransaction" --type ts --type tsx -A 15Repository: TempleDAO/temple
Length of output: 87
🏁 Script executed:
rg "SafeMultisigTransaction" -A 8 | head -100Repository: TempleDAO/temple
Length of output: 7268
🏁 Script executed:
rg "v1SafesMultisigTransactionsCreate" -B 3 -A 3Repository: TempleDAO/temple
Length of output: 487
🏁 Script executed:
fd "api" -e json | head -20Repository: TempleDAO/temple
Length of output: 98
🏁 Script executed:
rg "interface.*SafeMultisigTransaction|type.*SafeMultisigTransaction" -A 15Repository: TempleDAO/temple
Length of output: 1395
🏁 Script executed:
cat apps/dapp/src/safe/open-api/apiV1.json | head -200Repository: TempleDAO/temple
Length of output: 5222
🏁 Script executed:
fd "client" apps/dapp/src/safe/open-api -type fRepository: TempleDAO/temple
Length of output: 230
🏁 Script executed:
rg "export.*type SafeMultisigTransaction|export.*interface SafeMultisigTransaction" -A 20Repository: TempleDAO/temple
Length of output: 42
🏁 Script executed:
rg "SafeMultisigTransaction" apps/dapp/src/safe/open-api/apiV1.json -A 30 | head -150Repository: TempleDAO/temple
Length of output: 4254
🏁 Script executed:
find apps/dapp/src/safe/open-api -type f -name "*.ts" -o -name "*.js" -o -name "*.tsx"Repository: TempleDAO/temple
Length of output: 109
🏁 Script executed:
cat apps/dapp/src/safe/open-api/apiV1.json | python3 << 'EOF'
import json
import sys
try:
with open('apps/dapp/src/safe/open-api/apiV1.json', 'r') as f:
spec = json.load(f)
# Look for SafeMultisigTransaction in definitions
if 'definitions' in spec:
if 'SafeMultisigTransaction' in spec['definitions']:
print("=== SafeMultisigTransaction Definition ===")
print(json.dumps(spec['definitions']['SafeMultisigTransaction'], indent=2))
else:
print("SafeMultisigTransaction not found in definitions")
print("Available definitions:", list(spec['definitions'].keys())[:10])
except Exception as e:
print(f"Error: {e}")
EOFRepository: TempleDAO/temple
Length of output: 146
🏁 Script executed:
rg "\"SafeMultisigTransaction\"" apps/dapp/src/safe/open-api/apiV1.json -B 2 -A 50 | grep -A 50 "definitions" | head -100Repository: TempleDAO/temple
Length of output: 42
🏁 Script executed:
grep -n "definitions" apps/dapp/src/safe/open-api/apiV1.json | head -5Repository: TempleDAO/temple
Length of output: 342
🏁 Script executed:
tail -5000 apps/dapp/src/safe/open-api/apiV1.json | grep -A 50 "SafeMultisigTransaction"Repository: TempleDAO/temple
Length of output: 13252
🏁 Script executed:
find apps/dapp/src/safe -name "*client*" -o -name "*generated*"Repository: TempleDAO/temple
Length of output: 42
🏁 Script executed:
rg "from.*safe/open-api/client" apps/dapp/src/safe -lRepository: TempleDAO/temple
Length of output: 99
🏁 Script executed:
find apps/dapp/src/safe -name "client*" -o -name "index*" -o -name "*export*"Repository: TempleDAO/temple
Length of output: 42
🏁 Script executed:
ls -la apps/dapp/src/safe/open-api/Repository: TempleDAO/temple
Length of output: 440
🏁 Script executed:
wc -l apps/dapp/src/safe/open-api/apiV1.jsonRepository: TempleDAO/temple
Length of output: 103
🏁 Script executed:
rg "import.*SafeMultisigTransaction.*from" apps/dapp/src/safe -B 2 -A 2Repository: TempleDAO/temple
Length of output: 42
🏁 Script executed:
grep -r "export.*SafeMultisigTransaction" apps/dapp/src/safe/Repository: TempleDAO/temple
Length of output: 148
🏁 Script executed:
cat apps/dapp/src/safe/open-api/.gitignoreRepository: TempleDAO/temple
Length of output: 66
🏁 Script executed:
cat apps/dapp/src/safe/open-api/use-safe-open-api.tsx | head -50Repository: TempleDAO/temple
Length of output: 1269
🏁 Script executed:
ls -la apps/dapp/src/safe/open-api/client/ 2>/dev/null | head -20Repository: TempleDAO/temple
Length of output: 42
🏁 Script executed:
cat apps/dapp/src/safe/open-api/client/index.ts 2>/dev/null | head -100Repository: TempleDAO/temple
Length of output: 42
🏁 Script executed:
find apps/dapp/src/safe/open-api/client -name "*.ts" 2>/dev/null | head -10Repository: TempleDAO/temple
Length of output: 42
🏁 Script executed:
grep -A 20 "export.*interface SafeMultisigTransaction" apps/dapp/src/safe/open-api/client/*.ts 2>/dev/null | head -80Repository: TempleDAO/temple
Length of output: 42
🏁 Script executed:
cat apps/dapp/src/safe/open-api/use-safe-open-api.tsx | grep -A 5 "from './client'"Repository: TempleDAO/temple
Length of output: 209
🏁 Script executed:
rg "SafeMultisigTransaction" apps/dapp/src/safe/open-api/ -A 5Repository: TempleDAO/temple
Length of output: 6471
🏁 Script executed:
cat package.json | grep -i "safe\|openapi" | head -20Repository: TempleDAO/temple
Length of output: 104
🏁 Script executed:
rg "export.*SafeMultisigTransaction" apps/dapp/ -A 10 | head -60Repository: TempleDAO/temple
Length of output: 854
🏁 Script executed:
rg "V1Service\.v1SafesMultisigTransactionsCreate" apps/dapp -B 5 -A 10 | head -80Repository: TempleDAO/temple
Length of output: 974
🏁 Script executed:
cat apps/dapp/src/safe/open-api/apiV1.json | python3 << 'EOF'
import json
with open('apps/dapp/src/safe/open-api/apiV1.json', 'r') as f:
spec = json.load(f)
# Compare SafeMultisigTransaction vs SafeMultisigTransactionResponse
print("=== SafeMultisigTransaction (INPUT) ===")
if 'SafeMultisigTransaction' in spec['definitions']:
smt = spec['definitions']['SafeMultisigTransaction']
print(f"value field: {json.dumps(smt['properties']['value'], indent=2)}")
print("\n=== SafeMultisigTransactionResponse (OUTPUT) ===")
if 'SafeMultisigTransactionResponse' in spec['definitions']:
smtr = spec['definitions']['SafeMultisigTransactionResponse']
print(f"value field: {json.dumps(smtr['properties']['value'], indent=2)}")
EOFRepository: TempleDAO/temple
Length of output: 296
🏁 Script executed:
cat apps/dapp/src/safe/sdk/use-safe-sdk.tsx | sed -n '260,300p'Repository: TempleDAO/temple
Length of output: 1426
The as unknown as number cast on value field creates a runtime type mismatch with the API contract.
The review comment correctly identifies a real issue, though the explanation in the code requires clarification:
-
Spec Definition Mismatch: The OpenAPI spec defines
SafeMultisigTransaction.valueas type"integer"(for input), not"number". The comment's claim about the spec is inaccurate. The"string"type appears only inSafeMultisigTransactionResponse(output schema), not input. -
Double Cast Doesn't Fix Runtime Type: The
as unknown as numbercast does not convert the actual runtime value. IftransactionData.valueis a string, it remains a string. JSON serialization will produce"123"(quoted) instead of123(unquoted integer) that the API input schema expects. -
Actual API Contract Risk: Sending a JSON-serialized string where the API expects an integer may cause validation failures, even if the response uses strings. This is a real API contract violation, not a safe workaround.
Recommended Fix:
- Validate that the API actually accepts string values for input (test against live service)
- If strings are rejected, convert to BigInt or store as string in a proper string field
- If accepted, update the TypeScript type definition to
value: stringto remove the type violation
🤖 Prompt for AI Agents
In @apps/dapp/src/safe/sdk/use-safe-sdk.tsx around lines 268-296, The value
field in the safeMultisigTx object is being force-cast via transactionData.value
as unknown as number which does not change the runtime type and can violate the
API input contract; replace this with a proper conversion or type update: (1)
confirm whether the Safe Transaction Service accepts string inputs by testing
the live API, (2) if the API requires a numeric integer, convert
transactionData.value to a numeric form (e.g., BigInt -> number-safe
serialization or integer) before assigning to value for
V1Service.v1SafesMultisigTransactionsCreate, or (3) if the API accepts strings,
update the TypeScript input type for SafeMultisigTransaction.value to string and
provide transactionData.value as a string to remove the unsafe cast; adjust
related type declarations and add validation/error handling around the
conversion.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @apps/dapp/src/components/Pages/Safe/admin/BatchAuctionForm.tsx:
- Around line 162-164: Wrap calls to parseUnits in try/catch to avoid
render-time exceptions: around the minAmountWei assignment (the expression using
minimumDistributedAmount and parseUnits) catch errors and fall back to a safe
string like '0' (or mark invalid input) so the component doesn't crash; apply
the same pattern inside renderFundTab where parseUnits is used (the same
minimumDistributedAmount → parseUnits usage) to validate/guard input before
converting. Ensure you reference and protect the parseUnits usages that compute
minAmountWei and the analogous variable in renderFundTab.
🧹 Nitpick comments (3)
apps/dapp/src/components/Pages/Safe/admin/BatchAuctionForm.tsx (3)
27-31: Unused constants detected.
AUCTION_OPTIONSandBID_TOKEN_OPTIONSare defined at module level but never used in the component. The component uses staticStaticFieldelements with hardcoded "ENA Auction" labels instead of these options.Either remove these unused constants or integrate them into the UI for dynamic auction/token selection as originally intended.
79-84: Add input validation beforeparseUnitsto prevent runtime exceptions.
parseUnitswill throw ifminimumDistributedAmountcontains invalid input (e.g., negative values, non-numeric characters, or values with too many decimal places). The same applies totokenAmounton line 110.Consider adding validation before encoding:
🔎 Proposed validation
+ // Validate numeric inputs + const durationNum = Number(duration); + const waitPeriodNum = Number(waitPeriod); + if (isNaN(durationNum) || isNaN(waitPeriodNum) || durationNum < 0 || waitPeriodNum < 0) { + throw new Error('Duration and wait period must be valid positive numbers'); + } + const configData = spiceAuctionInterface.encodeFunctionData( 'setAuctionConfig', [ { - duration: Number(duration), - waitPeriod: Number(waitPeriod), + duration: durationNum, + waitPeriod: waitPeriodNum, minimumDistributedAuctionToken: parseUnits( minimumDistributedAmount, 18 ).toString(),
371-377: Remove unused styled components.The following styled components are defined but never used in the component:
Title(lines 371-377)CheckboxContainer(lines 452-456)Checkbox(lines 458-462)CheckboxLabel(lines 464-467)These appear to be remnants from an earlier design. Remove them to reduce code clutter.
Also applies to: 452-467
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/dapp/src/components/Pages/Safe/admin/BatchAuctionForm.tsxapps/dapp/src/constants/env/local.tsxapps/dapp/src/constants/env/preview.tsxapps/dapp/src/constants/env/production.tsxapps/dapp/src/constants/env/types.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-30T23:20:17.523Z
Learnt from: frontier159
Repo: TempleDAO/temple PR: 1214
File: protocol/scripts/deploys/mainnet/team-payments/deploy.ts:9-9
Timestamp: 2025-08-30T23:20:17.523Z
Learning: In Temple DAO's protocol/scripts/deploys/mainnet/team-payments/deploy.ts, the user prefers to keep hard-coded imports for epoch snapshot JSON files (like epoch29b.json) rather than parametrizing them. Avoid suggesting refactoring to make snapshot files runtime parameters.
Applied to files:
apps/dapp/src/constants/env/production.tsxapps/dapp/src/constants/env/preview.tsx
📚 Learning: 2025-11-20T01:06:33.012Z
Learnt from: marshall2112
Repo: TempleDAO/temple PR: 1224
File: apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/MyActivity/BidsForSpice/DataTables/BidDataTableMobile.tsx:238-243
Timestamp: 2025-11-20T01:06:33.012Z
Learning: In this Temple DAO codebase, the Button component (apps/dapp/src/components/Button/Button.tsx) has built-in async operation handling. It manages loading state internally, prevents double-clicks, awaits async onClick handlers, handles errors with try/catch, and displays a Loader component during operations. Components using styled(Button) like TradeButton inherit this functionality, so additional loading state management is not needed when onClick handlers return Promises.
Applied to files:
apps/dapp/src/components/Pages/Safe/admin/BatchAuctionForm.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Dapp Build
- GitHub Check: vercel-deploy
🔇 Additional comments (5)
apps/dapp/src/constants/env/local.tsx (1)
158-167: LGTM!Feature flags and placeholder addresses for local development are appropriate. The
'0x0'values clearly indicate these need to be configured for actual use.apps/dapp/src/constants/env/types.ts (1)
143-152: LGTM!Type definitions for the new feature flags and
spiceAuctionAdminconfiguration are well-structured and align with the implementations across the environment config files.apps/dapp/src/constants/env/production.tsx (1)
361-370: LGTM!Production configuration is appropriate with
enableGnosisSafeAdmin: falseandenableSpiceAuctionAdmin: true. Mainnet addresses are properly configured.apps/dapp/src/components/Pages/Safe/admin/BatchAuctionForm.tsx (2)
260-262: LGTM!The start time validation correctly checks for past dates, displays a warning message, and disables the submit button. This is a good UX pattern for preventing user errors.
Also applies to: 309-314, 333-335
35-47: LGTM!The component structure is clean with proper separation of concerns between tabs, appropriate use of context hooks for Safe transactions, wallet, and notifications. The tab switching implementation is straightforward and maintainable.
Also applies to: 342-363
| const minAmountWei = minimumDistributedAmount | ||
| ? parseUnits(minimumDistributedAmount, 18).toString() | ||
| : '0'; |
There was a problem hiding this comment.
Wrap parseUnits in try/catch to prevent render crashes.
If the user enters an invalid value (e.g., "abc" or "-1"), parseUnits will throw and crash the component during render. The same issue exists in renderFundTab at lines 254-256.
🔎 Proposed fix
- const minAmountWei = minimumDistributedAmount
- ? parseUnits(minimumDistributedAmount, 18).toString()
- : '0';
+ let minAmountWei = '0';
+ try {
+ if (minimumDistributedAmount) {
+ minAmountWei = parseUnits(minimumDistributedAmount, 18).toString();
+ }
+ } catch {
+ minAmountWei = 'Invalid';
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const minAmountWei = minimumDistributedAmount | |
| ? parseUnits(minimumDistributedAmount, 18).toString() | |
| : '0'; | |
| let minAmountWei = '0'; | |
| try { | |
| if (minimumDistributedAmount) { | |
| minAmountWei = parseUnits(minimumDistributedAmount, 18).toString(); | |
| } | |
| } catch { | |
| minAmountWei = 'Invalid'; | |
| } |
🤖 Prompt for AI Agents
In @apps/dapp/src/components/Pages/Safe/admin/BatchAuctionForm.tsx around lines
162 - 164, Wrap calls to parseUnits in try/catch to avoid render-time
exceptions: around the minAmountWei assignment (the expression using
minimumDistributedAmount and parseUnits) catch errors and fall back to a safe
string like '0' (or mark invalid input) so the component doesn't crash; apply
the same pattern inside renderFundTab where parseUnits is used (the same
minimumDistributedAmount → parseUnits usage) to validate/guard input before
converting. Ensure you reference and protect the parseUnits usages that compute
minAmountWei and the analogous variable in renderFundTab.
Description
What does this PR solve?
Introduces a new Batch Txn form for helping with Spice Auction config
Checklist
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.