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.

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

Refactor(Tx flow): simplify extractTxInfo #4876

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const SIGN_EXECUTE_TEXT = 'Sign or immediately execute this transaction.'

const ConfirmProposedTx = ({ txSummary }: ConfirmProposedTxProps): ReactElement => {
const signer = useSigner()
const { safe, safeAddress } = useSafeInfo()
const { safe } = useSafeInfo()
const chainId = useChainId()
const { setSafeTx, setSafeTxError, setNonce } = useContext(SafeTxContext)

Expand All @@ -33,8 +33,8 @@ const ConfirmProposedTx = ({ txSummary }: ConfirmProposedTxProps): ReactElement
}, [setNonce, txNonce])

useEffect(() => {
createExistingTx(chainId, safeAddress, txId).then(setSafeTx).catch(setSafeTxError)
}, [txId, safeAddress, chainId, setSafeTx, setSafeTxError])
createExistingTx(chainId, txId).then(setSafeTx).catch(setSafeTxError)
Copy link
Contributor

Choose a reason for hiding this comment

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

nice, so bsaically I can just use this createExistingTx in th app as well.

}, [txId, chainId, setSafeTx, setSafeTxError])

const text = canSign ? (canExecute ? SIGN_EXECUTE_TEXT : SIGN_TEXT) : EXECUTE_TEXT

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { DataDecoded, TransactionDetails } from '@safe-global/safe-gateway-typescript-sdk'
import { Box } from '@mui/material'
import useSafeInfo from '@/hooks/useSafeInfo'
import extractTxInfo from '@/services/tx/extractTxInfo'
import { isCustomTxInfo, isNativeTokenTransfer, isTransferTxInfo } from '@/utils/transaction-guards'
import SingleTxDecoded from '@/components/transactions/TxDetails/TxData/DecodedData/SingleTxDecoded'
Expand All @@ -11,7 +10,6 @@ import { type AccordionProps } from '@mui/material/Accordion/Accordion'

const DecodedTxs = ({ txs }: { txs: TransactionDetails[] | undefined }) => {
const [openMap, setOpenMap] = useState<Record<number, boolean>>()
const { safeAddress } = useSafeInfo()

if (!txs) return null

Expand All @@ -30,7 +28,7 @@ const DecodedTxs = ({ txs }: { txs: TransactionDetails[] | undefined }) => {
}))
}

const { txParams } = extractTxInfo(transaction, safeAddress)
const { txParams } = extractTxInfo(transaction)

let decodedDataParams: DataDecoded = {
method: '',
Expand Down
10 changes: 4 additions & 6 deletions apps/web/src/features/speedup/components/SpeedUpModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,10 @@ export const SpeedUpModal = ({
const dispatch = useAppDispatch()
const [trigger] = useLazyGetTransactionDetailsQuery()
const isDisabled = waitingForConfirmation || !wallet || !speedUpFee || !onboard
const [safeTx] = useAsync(async () => {
if (!chainInfo?.chainId || !safeAddress) {
return null
}
return createExistingTx(chainInfo.chainId, safeAddress, txId)
}, [txId, chainInfo?.chainId, safeAddress])
const [safeTx] = useAsync(() => {
if (!chainInfo?.chainId) return
return createExistingTx(chainInfo.chainId, txId)
}, [txId, chainInfo?.chainId])

const safeTxHasSignatures = !!safeTx?.signatures?.size ? true : false

Expand Down
6 changes: 2 additions & 4 deletions apps/web/src/features/speedup/hooks/useSafeTransaction.tsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,20 @@
import { useSafeSDK } from '@/hooks/coreSDK/safeCoreSDK'
import useChainId from '@/hooks/useChainId'
import useSafeAddress from '@/hooks/useSafeAddress'
import { useEffect, useState } from 'react'
import type { SafeTransaction } from '@safe-global/safe-core-sdk-types'
import { createExistingTx } from '@/services/tx/tx-sender'

export const useSafeTransaction = (txId: string) => {
const safeSdk = useSafeSDK()
const chainId = useChainId()
const safeAddress = useSafeAddress()
const [safeTx, setSafeTx] = useState<SafeTransaction>()

useEffect(() => {
if (!safeSdk) {
return
}
createExistingTx(chainId, safeAddress, txId).then(setSafeTx)
}, [chainId, safeAddress, txId, safeSdk])
createExistingTx(chainId, txId).then(setSafeTx)
}, [chainId, txId, safeSdk])

return safeTx
}
14 changes: 5 additions & 9 deletions apps/web/src/services/tx/__tests__/extractTxInfo.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ describe('extractTxInfo', () => {
const txDetails = {
txData: {
operation: 'CALL',
to: { value: '0x1234567890123456789012345678901234567890' },
value: '1000000000000000000',
data: '0x1234567890123456789012345678901234567890',
},
Expand Down Expand Up @@ -36,9 +37,7 @@ describe('extractTxInfo', () => {
},
} as unknown as TransactionDetails

const safeAddress = '0x1234567890123456789012345678901234567890'

expect(extractTxInfo(txDetails, safeAddress)).toEqual({
expect(extractTxInfo(txDetails)).toEqual({
txParams: {
data: '0x',
baseGas: '21000',
Expand All @@ -60,6 +59,7 @@ describe('extractTxInfo', () => {
it('should extract tx info for an ERC20 token transfer', () => {
const txDetails = {
txData: {
to: { value: '0xa74476443119A942dE498590Fe1f2454d7D4aC0d' },
operation: 'CALL',
value: '0x0',
hexData: '0x546785',
Expand Down Expand Up @@ -92,9 +92,7 @@ describe('extractTxInfo', () => {
},
} as unknown as TransactionDetails

const safeAddress = '0x1234567890123456789012345678901234567890'

expect(extractTxInfo(txDetails, safeAddress)).toEqual({
expect(extractTxInfo(txDetails)).toEqual({
txParams: {
data: '0x546785',
baseGas: '21000',
Expand Down Expand Up @@ -201,9 +199,7 @@ describe('extractTxInfo', () => {
},
} as unknown as TransactionDetails

const safeAddress = '0xF979f34D16d865f51e2eC7baDEde4f3735DaFb7d'

expect(extractTxInfo(txDetails, safeAddress)).toEqual({
expect(extractTxInfo(txDetails)).toEqual({
signatures: {
'0xbbeedB6d8e56e23f5812e59d1b6602F15957271F':
'0xb10e0605bd27c42af87ff36d690a5594d13a4a7029ea5f080dde917d0781f97901958195d0d99c7805419adb636ccdc579e9aa200ee0443dd10c4f5885f37f081b',
Expand Down
125 changes: 26 additions & 99 deletions apps/web/src/services/tx/extractTxInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,115 +2,42 @@ import type { OperationType } from '@safe-global/safe-core-sdk-types'
import { type SafeTransactionData } from '@safe-global/safe-core-sdk-types'
import type { TransactionDetails } from '@safe-global/safe-gateway-typescript-sdk'
import { Operation } from '@safe-global/safe-gateway-typescript-sdk'
import { isMultisigDetailedExecutionInfo, isNativeTokenTransfer } from '@/utils/transaction-guards'
import { isMultisigDetailedExecutionInfo } from '@/utils/transaction-guards'

const ZERO_ADDRESS: string = '0x0000000000000000000000000000000000000000'
const EMPTY_DATA: string = '0x'

/**
* Convert the CGW tx type to a Safe Core SDK tx
*/
const extractTxInfo = (
txDetails: TransactionDetails,
safeAddress: string,
): { txParams: SafeTransactionData; signatures: Record<string, string> } => {
// Format signatures into a map
let signatures: Record<string, string> = {}
if (isMultisigDetailedExecutionInfo(txDetails.detailedExecutionInfo)) {
signatures = txDetails.detailedExecutionInfo.confirmations.reduce((result, item) => {
result[item.signer.value] = item.signature ?? ''
return result
}, signatures)
}

const data = txDetails.txData?.hexData ?? EMPTY_DATA

const baseGas = isMultisigDetailedExecutionInfo(txDetails.detailedExecutionInfo)
? txDetails.detailedExecutionInfo.baseGas
: '0'

const gasPrice = isMultisigDetailedExecutionInfo(txDetails.detailedExecutionInfo)
? txDetails.detailedExecutionInfo.gasPrice
: '0'

const safeTxGas = isMultisigDetailedExecutionInfo(txDetails.detailedExecutionInfo)
? txDetails.detailedExecutionInfo.safeTxGas
: '0'
const execInfo = isMultisigDetailedExecutionInfo(txDetails.detailedExecutionInfo)
? txDetails.detailedExecutionInfo
: undefined
const txData = txDetails?.txData

const gasToken = isMultisigDetailedExecutionInfo(txDetails.detailedExecutionInfo)
? txDetails.detailedExecutionInfo.gasToken
: ZERO_ADDRESS

const nonce = isMultisigDetailedExecutionInfo(txDetails.detailedExecutionInfo)
? txDetails.detailedExecutionInfo.nonce
: 0

const refundReceiver = isMultisigDetailedExecutionInfo(txDetails.detailedExecutionInfo)
? txDetails.detailedExecutionInfo.refundReceiver.value
: ZERO_ADDRESS

const value = (() => {
switch (txDetails.txInfo.type) {
case 'Transfer':
if (isNativeTokenTransfer(txDetails.txInfo.transferInfo)) {
return txDetails.txInfo.transferInfo.value
} else {
return txDetails.txData?.value ?? '0'
}
case 'TwapOrder':
return txDetails.txData?.value ?? '0'
case 'SwapOrder':
return txDetails.txData?.value ?? '0'
case 'NativeStakingDeposit':
case 'NativeStakingValidatorsExit':
case 'NativeStakingWithdraw':
return txDetails.txData?.value ?? '0'
case 'Custom':
return txDetails.txInfo.value
case 'Creation':
case 'SettingsChange':
return '0'
default: {
throw new Error(`Unknown transaction type: ${txDetails.txInfo.type}`)
}
}
})()

const to = (() => {
switch (txDetails.txInfo.type) {
case 'Transfer':
if (isNativeTokenTransfer(txDetails.txInfo.transferInfo)) {
return txDetails.txInfo.recipient.value
} else {
return txDetails.txInfo.transferInfo.tokenAddress
}
case 'SwapOrder':
case 'TwapOrder':
const orderTo = txDetails.txData?.to.value
if (!orderTo) {
throw new Error('Order tx data does not have a `to` field')
}
return orderTo
case 'NativeStakingDeposit':
case 'NativeStakingValidatorsExit':
case 'NativeStakingWithdraw':
const stakingTo = txDetails.txData?.to.value
if (!stakingTo) {
throw new Error('Staking tx data does not have a `to` field')
}
return stakingTo
case 'Custom':
return txDetails.txInfo.to.value
case 'Creation':
case 'SettingsChange':
return safeAddress
default: {
throw new Error(`Unknown transaction type: ${txDetails.txInfo.type}`)
}
}
})()

const operation = (txDetails.txData?.operation ?? Operation.CALL) as unknown as OperationType
// Format signatures into a map
const signatures =
execInfo?.confirmations.reduce(
(result, item) => {
result[item.signer.value] = item.signature ?? ''
return result
},
{} as Record<string, string>,
) ?? {}

const nonce = execInfo?.nonce ?? 0
const baseGas = execInfo?.baseGas ?? '0'
const gasPrice = execInfo?.gasPrice ?? '0'
const safeTxGas = execInfo?.safeTxGas ?? '0'
const gasToken = execInfo?.gasToken ?? ZERO_ADDRESS
const refundReceiver = execInfo?.refundReceiver.value ?? ZERO_ADDRESS

const to = txData?.to.value ?? ZERO_ADDRESS
const value = txData?.value ?? '0'
const data = txData?.hexData ?? '0x'
const operation = (txData?.operation ?? Operation.CALL) as unknown as OperationType

return {
txParams: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ describe('txSender', () => {

describe('createExistingTx', () => {
it('should create a tx from an existing proposal', async () => {
const tx = await createExistingTx('4', '0x123', '0x345')
const tx = await createExistingTx('4', '0x345')

expect(getTransactionDetails).toHaveBeenCalledWith('4', '0x345')
expect(extractTxInfo).toHaveBeenCalled()
Expand Down
3 changes: 1 addition & 2 deletions apps/web/src/services/tx/tx-sender/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,14 @@ export const createRejectTx = async (nonce: number): Promise<SafeTransaction> =>
*/
export const createExistingTx = async (
chainId: string,
safeAddress: string,
txId: string,
txDetails?: TransactionDetails,
): Promise<SafeTransaction> => {
// Get the tx details from the backend if not provided
txDetails = txDetails || (await getTransactionDetails(chainId, txId))

// Convert them to the Core SDK tx params
const { txParams, signatures } = extractTxInfo(txDetails, safeAddress)
const { txParams, signatures } = extractTxInfo(txDetails)

// Create a tx and add pre-approved signatures
const safeTx = await createTx(txParams, txParams.nonce)
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/services/tx/tx-sender/dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ export const dispatchSafeTxSpeedUp = async (
// Execute the tx
let result: TransactionResult | undefined
try {
const safeTx = await createExistingTx(chainId, safeAddress, txId)
const safeTx = await createExistingTx(chainId, txId)

// TODO: This is a workaround until there is a fix for unchecked transactions in the protocol-kit
if (isSmartAccount) {
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/utils/transactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export const getMultiSendTxs = async (
.map((tx) => {
if (!isMultisigDetailedExecutionInfo(tx.detailedExecutionInfo)) return

const args = extractTxInfo(tx, safeAddress)
const args = extractTxInfo(tx)
const sigs = getSignatures(args.signatures)

// @ts-ignore
Expand Down
Loading