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

feat(api-kit): Support for SafeOperation confirmations endpoints #876

Merged
merged 15 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 12 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
70 changes: 66 additions & 4 deletions packages/api-kit/src/SafeApiKit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
AllTransactionsOptions,
DeleteSafeDelegateProps,
GetSafeDelegateProps,
GetSafeMessageListProps,
GetSafeOperationConfirmationListProps,
GetSafeOperationListProps,
GetSafeOperationListResponse,
SafeSingletonResponse,
GetSafeMessageListProps,
ModulesResponse,
OwnerResponse,
ProposeTransactionProps,
Expand All @@ -24,6 +24,7 @@
SafeMultisigTransactionEstimateResponse,
SafeMultisigTransactionListResponse,
SafeServiceInfoResponse,
SafeSingletonResponse,
SignatureResponse,
TokenInfoListResponse,
TokenInfoResponse,
Expand All @@ -34,11 +35,12 @@
import { validateEip3770Address, validateEthereumAddress } from '@safe-global/protocol-kit'
import {
Eip3770Address,
isSafeOperation,
SafeMultisigConfirmationListResponse,
SafeMultisigTransactionResponse,
SafeOperationResponse,
SafeOperation,
isSafeOperation
SafeOperationConfirmationListResponse,
SafeOperationResponse
} from '@safe-global/safe-core-sdk-types'
import { TRANSACTION_SERVICE_URLS } from './utils/config'
import { isEmptyData } from './utils'
Expand Down Expand Up @@ -118,7 +120,7 @@
* @throws "Not Found"
* @throws "Ensure this field has at least 1 hexadecimal chars (not counting 0x)."
*/
async decodeData(data: string): Promise<any> {

Check warning on line 123 in packages/api-kit/src/SafeApiKit.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected any. Specify a different type
if (data === '') {
throw new Error('Invalid data')
}
Expand Down Expand Up @@ -328,7 +330,7 @@
const { address: delegator } = this.#getEip3770Address(delegatorAddress)
const signature = await signDelegate(signer, delegate, this.#chainId)

const body: any = {

Check warning on line 333 in packages/api-kit/src/SafeApiKit.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected any. Specify a different type
safe: safeAddress ? this.#getEip3770Address(safeAddress).address : null,
delegate,
delegator,
Expand Down Expand Up @@ -852,6 +854,66 @@
}
})
}

/**
* Returns the list of confirmations for a given a SafeOperation.
*
* @param getSafeOperationConfirmationsProps - The parameters for fetching the list of confirmations
* @returns The list of confirmations
* @throws "Invalid SafeOperation hash"
* @throws "Invalid data"
*/
async getSafeOperationConfirmations({
Copy link
Contributor

Choose a reason for hiding this comment

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

Please double check how we do this. This kind of looks like a getSafeOperationConfirmationsByHash with a different name as the hash itself is a part of a "perma-link". In this case, it would be common to separate the interface as getSafeOperationConfirmations(hash, { limit, offset }) since that would be somehow associated with a domain model.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the function signature accordingly 👍

safeOperationHash,
limit,
offset
}: GetSafeOperationConfirmationListProps): Promise<SafeOperationConfirmationListResponse> {
if (!safeOperationHash) {
throw new Error('Invalid SafeOperation hash')
}

const url = new URL(
`${this.#txServiceBaseUrl}/v1/safe-operations/${safeOperationHash}/confirmations/`
)

if (limit) {
url.searchParams.set('limit', limit)
}

if (offset) {
url.searchParams.set('offset', offset)
}

return sendRequest({
url: url.toString(),
method: HttpMethod.Get
})
}

/**
* Adds a confirmation for a SafeOperation.
*
* @param safeOperationHash The SafeOperation hash
* @param signature - Signature of the SafeOperation
* @returns
* @throws "Invalid SafeOperation hash"
* @throws "Invalid signature"
* @throws "Malformed data"
* @throws "Error processing data"
*/
async confirmSafeOperation(safeOperationHash: string, signature: string): Promise<void> {
if (!safeOperationHash) {
throw new Error('Invalid SafeOperation hash')
}
if (!signature) {
throw new Error('Invalid signature')
}
return sendRequest({
url: `${this.#txServiceBaseUrl}/v1/safe-operations/${safeOperationHash}/confirmations/`,
method: HttpMethod.Post,
body: { signature }
})
}
}

export default SafeApiKit
86 changes: 28 additions & 58 deletions packages/api-kit/src/types/safeTransactionServiceTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import {
SafeMultisigTransactionResponse,
SafeTransactionData,
UserOperation,
SafeOperationResponse
SafeOperationResponse,
ListResponse
} from '@safe-global/safe-core-sdk-types'

export type SafeServiceInfoResponse = {
Expand Down Expand Up @@ -89,17 +90,12 @@ export type SafeDelegateResponse = {
readonly signature: string
}

export type SafeDelegateListResponse = {
readonly count: number
readonly next?: string
readonly previous?: string
readonly results: {
readonly safe: string
readonly delegate: string
readonly delegator: string
readonly label: string
}[]
}
export type SafeDelegateListResponse = ListResponse<{
readonly safe: string
readonly delegate: string
readonly delegator: string
readonly label: string
}>

export type SafeMultisigTransactionEstimate = {
readonly to: string
Expand All @@ -125,12 +121,7 @@ export type ProposeTransactionProps = {
origin?: string
}

export type SafeMultisigTransactionListResponse = {
readonly count: number
readonly next?: string
readonly previous?: string
readonly results: SafeMultisigTransactionResponse[]
}
export type SafeMultisigTransactionListResponse = ListResponse<SafeMultisigTransactionResponse>

export type TransferResponse = {
readonly type?: string
Expand All @@ -144,12 +135,7 @@ export type TransferResponse = {
readonly from: string
}

export type TransferListResponse = {
readonly count: number
readonly next?: string
readonly previous?: string
readonly results: TransferResponse[]
}
export type TransferListResponse = ListResponse<TransferResponse>

export type SafeModuleTransaction = {
readonly created?: string
Expand All @@ -166,12 +152,7 @@ export type SafeModuleTransaction = {
readonly dataDecoded?: string
}

export type SafeModuleTransactionListResponse = {
readonly count: number
readonly next?: string
readonly previous?: string
readonly results: SafeModuleTransaction[]
}
export type SafeModuleTransactionListResponse = ListResponse<SafeModuleTransaction>

export type Erc20Info = {
readonly name: string
Expand All @@ -189,12 +170,7 @@ export type TokenInfoResponse = {
readonly logoUri?: string
}

export type TokenInfoListResponse = {
readonly count: number
readonly next?: string
readonly previous?: string
readonly results: TokenInfoResponse[]
}
export type TokenInfoListResponse = ListResponse<TokenInfoResponse>

export type TransferWithTokenInfoResponse = TransferResponse & {
readonly tokenInfo: TokenInfoResponse
Expand Down Expand Up @@ -230,16 +206,11 @@ export type AllTransactionsOptions = {
trusted?: boolean
}

export type AllTransactionsListResponse = {
readonly count: number
readonly next?: string
readonly previous?: string
readonly results: Array<
| SafeModuleTransactionWithTransfersResponse
| SafeMultisigTransactionWithTransfersResponse
| EthereumTxWithTransfersResponse
>
}
export type AllTransactionsListResponse = ListResponse<
| SafeModuleTransactionWithTransfersResponse
| SafeMultisigTransactionWithTransfersResponse
| EthereumTxWithTransfersResponse
>

export type ModulesResponse = {
safes: string[]
Expand All @@ -265,12 +236,7 @@ export type SafeMessage = {
readonly preparedSignature: string
}

export type SafeMessageListResponse = {
readonly count: number
readonly next?: string
readonly previous?: string
readonly results: SafeMessage[]
}
export type SafeMessageListResponse = ListResponse<SafeMessage>

export type AddMessageProps = {
message: string | EIP712TypedData
Expand Down Expand Up @@ -301,12 +267,7 @@ export type GetSafeOperationListProps = {
offset?: string
}

export type GetSafeOperationListResponse = {
readonly count: number
readonly next?: string
readonly previous?: string
readonly results: Array<SafeOperationResponse>
}
export type GetSafeOperationListResponse = ListResponse<SafeOperationResponse>

export type AddSafeOperationProps = {
/** Address of the EntryPoint contract */
Expand All @@ -325,3 +286,12 @@ export type AddSafeOperationProps = {
validAfter?: number
}
}

export type GetSafeOperationConfirmationListProps = {
Copy link
Contributor

Choose a reason for hiding this comment

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

It is a little bit weird to use limit and offset as a string since they are associated with record count in a database. I imagine that the interface will be something like apiKit.getSafeOperationConfirmations({ safeOperationHash, limit: '10' }).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes good point. Changed the type to number

/** The hash of the SafeOperation to get confirmations for */
safeOperationHash: string
/** Maximum number of results to return per page */
limit?: string
/** Initial index from which to return the results */
offset?: string
}
6 changes: 3 additions & 3 deletions packages/api-kit/tests/e2e/addSafeOperation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { ethers } from 'ethers'
import sinon from 'sinon'
import sinonChai from 'sinon-chai'
import Safe from '@safe-global/protocol-kit'
import SafeApiKit from '@safe-global/api-kit'
import SafeApiKit from '@safe-global/api-kit/index'
import { Safe4337Pack } from '@safe-global/relay-kit'
import { generateTransferCallData } from '@safe-global/relay-kit/src/packs/safe-4337/testing-utils/helpers'
import { generateTransferCallData } from '@safe-global/relay-kit/packs/safe-4337/testing-utils/helpers'
import { RPC_4337_CALLS } from '@safe-global/relay-kit/packs/safe-4337/constants'
import { getKits } from '../utils/setupKits'
import { getAddSafeOperationProps } from '@safe-global/api-kit/utils/safeOperation'
Expand All @@ -15,7 +15,7 @@ chai.use(chaiAsPromised)
chai.use(sinonChai)

const SIGNER_PK = '0x83a415ca62e11f5fa5567e98450d0f82ae19ff36ef876c10a8d448c788a53676'
const SAFE_ADDRESS = '0x60C4Ab82D06Fd7dFE9517e17736C2Dcc77443EF0' // 1/1 Safe (v1.4.1) with signer above as owner + 4337 module enabled
const SAFE_ADDRESS = '0x60C4Ab82D06Fd7dFE9517e17736C2Dcc77443EF0' // 1/2 Safe (v1.4.1) with signer above being an owner + 4337 module enabled
const PAYMASTER_TOKEN_ADDRESS = '0x1c7D4B196Cb0C7B01d743Fbc6116a902379C7238'
const PAYMASTER_ADDRESS = '0x0000000000325602a77416A16136FDafd04b299f'
const BUNDLER_URL = `https://bundler.url`
Expand Down
114 changes: 114 additions & 0 deletions packages/api-kit/tests/e2e/confirmSafeOperation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import chai from 'chai'
import chaiAsPromised from 'chai-as-promised'
import { Safe4337InitOptions, Safe4337Pack } from '@safe-global/relay-kit'
import { generateTransferCallData } from '@safe-global/relay-kit/packs/safe-4337/testing-utils/helpers'
import SafeApiKit from '@safe-global/api-kit/index'
import { getAddSafeOperationProps } from '@safe-global/api-kit/utils/safeOperation'
import { SafeOperation } from '@safe-global/safe-core-sdk-types'
import { getApiKit, getEip1193Provider } from '../utils/setupKits'

chai.use(chaiAsPromised)

const PRIVATE_KEY_1 = '0x83a415ca62e11f5fa5567e98450d0f82ae19ff36ef876c10a8d448c788a53676'
const PRIVATE_KEY_2 = '0xb88ad5789871315d0dab6fc5961d6714f24f35a6393f13a6f426dfecfc00ab44'
const SAFE_ADDRESS = '0x60C4Ab82D06Fd7dFE9517e17736C2Dcc77443EF0' // 4337 enabled 1/2 Safe (v1.4.1) owned by PRIVATE_KEY_1 + PRIVATE_KEY_2
const TX_SERVICE_URL = 'https://safe-transaction-sepolia.staging.5afe.dev/api'
const BUNDLER_URL = `https://bundler.url`
const PAYMASTER_TOKEN_ADDRESS = '0x1c7D4B196Cb0C7B01d743Fbc6116a902379C7238'

let safeApiKit: SafeApiKit
let safe4337Pack: Safe4337Pack
let safeOperation: SafeOperation
let safeOpHash: string

describe('confirmSafeOperation', () => {
const transferUSDC = {
to: PAYMASTER_TOKEN_ADDRESS,
data: generateTransferCallData(SAFE_ADDRESS, 100_000n),
value: '0',
operation: 0
}

const getSafe4337Pack = async (options: Partial<Safe4337InitOptions>) =>
Safe4337Pack.init({
provider: options.provider || getEip1193Provider(),
signer: options.signer || PRIVATE_KEY_1,
options: { safeAddress: SAFE_ADDRESS },
bundlerUrl: BUNDLER_URL
})

const createSignature = async (safeOperation: SafeOperation, signer: string) => {
const safe4337Pack = await getSafe4337Pack({ signer })
const signedSafeOperation = await safe4337Pack.signSafeOperation(safeOperation)
const signerAddress = await safe4337Pack.protocolKit.getSafeProvider().getSignerAddress()
return signedSafeOperation.getSignature(signerAddress!)
}

/**
* Add a new Safe operation to the transaction service.
* @returns Resolves with the signed Safe operation
*/
const addSafeOperation = async (): Promise<SafeOperation> => {
const safeOperation = await safe4337Pack.createTransaction({
transactions: [transferUSDC]
})

const signedSafeOperation = await safe4337Pack.signSafeOperation(safeOperation)
const addSafeOperationProps = await getAddSafeOperationProps(signedSafeOperation)

await safeApiKit.addSafeOperation(addSafeOperationProps)

return signedSafeOperation
}

before(async () => {
safe4337Pack = await getSafe4337Pack({ signer: PRIVATE_KEY_1 })
safeApiKit = getApiKit(TX_SERVICE_URL)

// Submit a new Safe operation to the transaction service
safeOperation = await addSafeOperation()
safeOpHash = await safeOperation.getHash()
})

describe('should fail', () => {
it('if SafeOperation hash is empty', async () => {
const signature = await createSignature(safeOperation, PRIVATE_KEY_2)
await chai
.expect(safeApiKit.confirmSafeOperation('', signature!.data))
.to.be.rejectedWith('Invalid SafeOperation hash')
})

it('if signature is empty', async () => {
await chai
.expect(safeApiKit.confirmSafeOperation(safeOpHash, ''))
.to.be.rejectedWith('Invalid signature')
})

it('if signature is invalid', async () => {
await chai
.expect(safeApiKit.confirmSafeOperation(safeOpHash, '0xInvalidSignature'))
.to.be.rejectedWith('Bad Request')
})
})

it('should allow to create and confirm a SafeOperation signature using a Safe signer', async () => {
const signerAddress1 = await safe4337Pack.protocolKit.getSafeProvider().getSignerAddress()

// Create a signature for the Safe operation using owner 2
const signatureSigner2 = await createSignature(safeOperation, PRIVATE_KEY_2)

// Add the second signature to the Safe operation
await chai.expect(safeApiKit.confirmSafeOperation(safeOpHash, signatureSigner2!.data)).to.be
.fulfilled

// Check that the Safe operation is now confirmed by both owners
const safeOperationResponse = await safeApiKit.getSafeOperation(safeOpHash)
chai.expect(safeOperationResponse.confirmations).to.have.lengthOf(2)

chai
.expect(safeOperationResponse.confirmations![0].signature)
.to.eq(safeOperation.getSignature(signerAddress1!)!.data)

chai.expect(safeOperationResponse.confirmations![1].signature).to.eq(signatureSigner2!.data)
})
})
2 changes: 1 addition & 1 deletion packages/api-kit/tests/e2e/getSafeOperation.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import SafeApiKit from '@safe-global/api-kit'
import SafeApiKit from '@safe-global/api-kit/index'
import chai from 'chai'
import chaiAsPromised from 'chai-as-promised'
import { getApiKit } from '../utils/setupKits'
Expand Down
Loading
Loading