Skip to content

Commit

Permalink
Restore getTransactionHash
Browse files Browse the repository at this point in the history
  • Loading branch information
yagopv committed Oct 27, 2023
1 parent 3fdb31a commit 144ec46
Show file tree
Hide file tree
Showing 14 changed files with 51 additions and 47 deletions.
4 changes: 2 additions & 2 deletions guides/integrating-the-safe-core-sdk.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,13 +207,13 @@ Once we have the Safe transaction object we can share it with the other owners o

- `safeAddress`: The Safe address.
- `safeTransactionData`: The `data` object inside the Safe transaction object returned from the method `createTransaction`.
- `safeTxHash`: The Safe transaction hash, calculated by calling the method `getHash` from the Protocol Kit.
- `safeTxHash`: The Safe transaction hash, calculated by calling the method `getTransactionHash` from the Protocol Kit.
- `senderAddress`: The Safe owner or delegate proposing the transaction.
- `senderSignature`: The signature generated by signing the `safeTxHash` with the `senderAddress`.
- `origin`: Optional string that allows to provide more information about the app proposing the transaction.

```js
const safeTxHash = await safeSdk.getHash(safeTransaction)
const safeTxHash = await safeSdk.getTransactionHash(safeTransaction)
const senderSignature = await safeSdk.signHash(safeTxHash)
await safeService.proposeTransaction({
safeAddress,
Expand Down
4 changes: 2 additions & 2 deletions packages/api-kit/tests/e2e/addMessage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const generateRandomUUID = (): string => {
const generateMessage = () => `${generateRandomUUID()}: I am the owner of the safe`
const safeAddress = '0x3296b3DD454B7c3912F7F477787B503918C50082'

describe('addMessage', () => {
describe.only('addMessage', () => {
before(async () => {
;({ safeApiKit, ethAdapter } = await getServiceClient(
'0x4f3edf983ac636a65a842ce7c78d9aa706d3b113bce9c46f30d7d21715b23b1d'
Expand Down Expand Up @@ -55,7 +55,7 @@ describe('addMessage', () => {

it('should allow to create a new off-chain message signed with EIP-191', async () => {
const rawMessage = generateMessage()
const messageHash = await protocolKit.getHash(rawMessage)
const messageHash = await protocolKit.hashSafeMessage(rawMessage)
const safeMessageHash = await protocolKit.getSafeMessageHash(messageHash)
const signature = await protocolKit.signHash(safeMessageHash)

Expand Down
2 changes: 1 addition & 1 deletion packages/api-kit/tests/e2e/addMessageSignature.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe('addMessageSignature', () => {

it('should allow to add a confirmation signature using a mix of EIP-191 and EIP-712', async () => {
const rawMessage = generateMessage()
const messageHash = await protocolKit1.getHash(rawMessage)
const messageHash = await protocolKit1.hashSafeMessage(rawMessage)
const safeMessageHash = await protocolKit1.getSafeMessageHash(messageHash)
const ethSign = await protocolKit1.signHash(safeMessageHash)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ describe('SafeMoneriumClient', () => {
data: txData
})

safeSdk.getHash = jest.fn().mockResolvedValueOnce('0xTransactionHash')
safeSdk.getTransactionHash = jest.fn().mockResolvedValueOnce('0xTransactionHash')
safeSdk.signHash = jest.fn().mockResolvedValueOnce('0xTransactionSignature')

jest.spyOn(SafeApiKit.prototype, 'getTransaction').mockResolvedValueOnce({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export class SafeMoneriumClient extends MoneriumClient {
}
})

const safeTxHash = await this.#safeSdk.getHash(safeTransaction)
const safeTxHash = await this.#safeSdk.getTransactionHash(safeTransaction)

const senderSignature = await this.#safeSdk.signHash(safeTxHash)

Expand Down
12 changes: 6 additions & 6 deletions packages/protocol-kit/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ To connect `owner2` to the Safe we need to create a new instance of the class `E
```js
const ethAdapterOwner2 = new EthersAdapter({ ethers, signerOrProvider: owner2 })
const safeSdk2 = await safeSdk.connect({ ethAdapter: ethAdapterOwner2, safeAddress })
const txHash = await safeSdk2.getHash(safeTransaction)
const txHash = await safeSdk2.getTransactionHash(safeTransaction)
const approveTxResponse = await safeSdk2.approveTransactionHash(txHash)
await approveTxResponse.transactionResponse?.wait()
```
Expand Down Expand Up @@ -633,7 +633,7 @@ const safeTransaction1 = await safeSdk.createTransaction({ safeTransactionData }
const safeTransaction2 = await copyTransaction(safeTransaction1)
```
### getHash
### getTransactionHash
Returns the transaction hash of a Safe transaction.
Expand All @@ -642,7 +642,7 @@ const safeTransactionData: SafeTransactionDataPartial = {
// ...
}
const safeTransaction = await safeSdk.createTransaction({ safeTransactionData })
const txHash = await safeSdk.getHash(safeTransaction)
const txHash = await safeSdk.getTransactionHash(safeTransaction)
```
### signHash
Expand All @@ -654,7 +654,7 @@ const safeTransactionData: SafeTransactionDataPartial = {
// ...
}
const safeTransaction = await safeSdk.createTransaction({ safeTransactionData })
const txHash = await safeSdk.getHash(safeTransaction)
const txHash = await safeSdk.getTransactionHash(safeTransaction)
const signature = await safeSdk.signHash(txHash)
```
Expand Down Expand Up @@ -701,7 +701,7 @@ const safeTransactionData: SafeTransactionDataPartial = {
// ...
}
const safeTransaction = await safeSdk.createTransaction({ safeTransactionData })
const txHash = await safeSdk.getHash(safeTransaction)
const txHash = await safeSdk.getTransactionHash(safeTransaction)
const txResponse = await safeSdk.approveTransactionHash(txHash)
await txResponse.transactionResponse?.wait()
```
Expand Down Expand Up @@ -743,7 +743,7 @@ const safeTransactionData: SafeTransactionDataPartial = {
// ...
}
const safeTransaction = await safeSdk.createTransaction({ safeTransactionData })
const txHash = await safeSdk.getHash(safeTransaction)
const txHash = await safeSdk.getTransactionHash(safeTransaction)
const ownerAddresses = await safeSdk.getOwnersWhoApprovedTx(txHash)
```
Expand Down
21 changes: 11 additions & 10 deletions packages/protocol-kit/src/Safe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
EthSafeSignature,
SAFE_FEATURES,
hasSafeFeature,
hashSafeMessage,
isMetaTransactionArray,
isSafeMultisigTransactionResponse,
sameString
Expand Down Expand Up @@ -491,19 +492,15 @@ class Safe {
/**
* Returns the transaction hash of a Safe transaction.
*
* @param txOrMessage - The Safe transaction or a raw message
* @param safeTransaction - The Safe transaction or a raw message
* @returns The hashed Safe transaction or message
*/
async getHash(txOrMessage: SafeTransaction | string): Promise<string> {
async getTransactionHash(safeTransaction: SafeTransaction): Promise<string> {
if (!this.#contractManager.safeContract) {
throw new Error('Safe is not deployed')
}

if (typeof txOrMessage === 'string') {
return ethers.hashMessage(txOrMessage) || ''
}

const safeTransactionData = txOrMessage.data
const safeTransactionData = safeTransaction.data
const txHash = await this.#contractManager.safeContract.getTransactionHash(safeTransactionData)

return txHash
Expand All @@ -529,6 +526,10 @@ class Safe {
return signature
}

hashSafeMessage(message: string | EIP712TypedData): string {
return hashSafeMessage(message)
}

/**
* Signs a transaction according to the EIP-712 using the current signer account.
*
Expand Down Expand Up @@ -607,7 +608,7 @@ class Safe {
if (!hasSafeFeature(SAFE_FEATURES.ETH_SIGN, safeVersion)) {
throw new Error('eth_sign is only supported by Safes >= v1.1.0')
}
const txHash = await this.getHash(transaction)
const txHash = await this.getTransactionHash(transaction)
signature = await this.signHash(txHash, isSmartContract)
}

Expand Down Expand Up @@ -965,7 +966,7 @@ class Safe {

const signedSafeTransaction = await this.copyTransaction(transaction)

const txHash = await this.getHash(signedSafeTransaction)
const txHash = await this.getTransactionHash(signedSafeTransaction)
const ownersWhoApprovedTx = await this.getOwnersWhoApprovedTx(txHash)
for (const owner of ownersWhoApprovedTx) {
signedSafeTransaction.addSignature(generatePreValidatedSignature(owner))
Expand Down Expand Up @@ -1012,7 +1013,7 @@ class Safe {

const signedSafeTransaction = await this.copyTransaction(transaction)

const txHash = await this.getHash(signedSafeTransaction)
const txHash = await this.getTransactionHash(signedSafeTransaction)
const ownersWhoApprovedTx = await this.getOwnersWhoApprovedTx(txHash)
for (const owner of ownersWhoApprovedTx) {
signedSafeTransaction.addSignature(generatePreValidatedSignature(owner))
Expand Down
2 changes: 1 addition & 1 deletion packages/protocol-kit/src/utils/eip-712/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const hashMessage = (message: string): string => {
return ethers.hashMessage(message)
}

const hashSafeMessage = (message: string | EIP712TypedData): string => {
export const hashSafeMessage = (message: string | EIP712TypedData): string => {
return typeof message === 'string' ? hashMessage(message) : hashTypedData(message)
}

Expand Down
19 changes: 11 additions & 8 deletions packages/protocol-kit/tests/e2e/eip1271.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { getAccounts } from './utils/setupTestNetwork'
import { waitSafeTxReceipt } from './utils/transactions'
import { itif } from './utils/helpers'
import { ethers } from 'ethers'
import { buildSignature } from '@safe-global/protocol-kit/utils'
import { buildSignature, hashSafeMessage } from '@safe-global/protocol-kit/utils'

chai.use(chaiAsPromised)

Expand Down Expand Up @@ -139,7 +139,7 @@ describe.only('EIP1271', () => {
customContractAbi: customContract.signMessageLibAbi
})

const messageHash = await safeSdk1.getHash(MESSAGE)
const messageHash = safeSdk1.hashSafeMessage(MESSAGE)

const txData = signMessageLibContract.encode('signMessage', [messageHash])

Expand Down Expand Up @@ -169,7 +169,7 @@ describe.only('EIP1271', () => {
const { safeSdk1, safeSdk2 } = await setupTests()

// Hash the message
const messageHash = await safeSdk1.getHash(MESSAGE)
const messageHash = await safeSdk1.hashSafeMessage(MESSAGE)
const safeMessageHash = await safeSdk1.getSafeMessageHash(messageHash)

// Sign the Safe message hash with the owners
Expand Down Expand Up @@ -201,7 +201,7 @@ describe.only('EIP1271', () => {
const { safeSdk1, safeSdk2 } = await setupTests()

// Hash the message
const messageHash = await safeSdk1.getHash(MESSAGE)
const messageHash = await safeSdk1.hashSafeMessage(MESSAGE)
const safeMessageHash = await safeSdk1.getSafeMessageHash(messageHash)

// Sign the Safe message with the owners
Expand All @@ -221,7 +221,7 @@ describe.only('EIP1271', () => {
const { safeSdk1, safeSdk2, safeSdk3 } = await setupTests()

// Hash the message
const messageHash = await safeSdk1.getHash(MESSAGE)
const messageHash = await safeSdk1.hashSafeMessage(MESSAGE)
const safeMessageHash = await safeSdk1.getSafeMessageHash(messageHash)

// Sign the Safe message with the owners
Expand All @@ -246,7 +246,10 @@ describe.only('EIP1271', () => {
itif(safeVersionDeployed >= '1.3.0')('should revert when message is not signed', async () => {
const { safeSdk1 } = await setupTests()

const response = await safeSdk1.isValidSignature(await safeSdk1.getHash(MESSAGE), '0x')
const response = await safeSdk1.isValidSignature(
await safeSdk1.hashSafeMessage(MESSAGE),
'0x'
)

chai.expect(response).to.be.false
})
Expand All @@ -257,7 +260,7 @@ describe.only('EIP1271', () => {
const { safeAddress, safeSdk1 } = await setupTests()

const chainId = await safeSdk1.getChainId()
const messageHash = await safeSdk1.getHash(MESSAGE)
const messageHash = await safeSdk1.hashSafeMessage(MESSAGE)
const safeMessageHash = await safeSdk1.getSafeMessageHash(messageHash)

chai
Expand Down Expand Up @@ -287,7 +290,7 @@ describe.only('EIP1271', () => {
}

const tx = await safeSdk1.createTransaction({ safeTransactionData })
const txHash = await safeSdk1.getHash(tx)
const txHash = await safeSdk1.getTransactionHash(tx)

const signature1 = await safeSdk1.signHash(await safeSdk1.getSafeMessageHash(txHash))
const signature2 = await safeSdk3.signHash(
Expand Down
16 changes: 8 additions & 8 deletions packages/protocol-kit/tests/e2e/execution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ describe('Transactions execution', () => {
}
const tx = await safeSdk1.createTransaction({ safeTransactionData })
const signedTx = await safeSdk1.signTransaction(tx)
const txHash = await safeSdk2.getHash(tx)
const txHash = await safeSdk2.getTransactionHash(tx)
const txResponse = await safeSdk2.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse)
await chai
Expand Down Expand Up @@ -328,7 +328,7 @@ describe('Transactions execution', () => {
const tx = await safeSdk1.createTransaction({ safeTransactionData })

// Signature: on-chain
const txHash = await safeSdk1.getHash(tx)
const txHash = await safeSdk1.getTransactionHash(tx)
const txResponse1 = await safeSdk1.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse1)

Expand Down Expand Up @@ -382,7 +382,7 @@ describe('Transactions execution', () => {
const tx = await safeSdk1.createTransaction({ safeTransactionData })

// Signature: on-chain
const txHash = await safeSdk1.getHash(tx)
const txHash = await safeSdk1.getTransactionHash(tx)
const txResponse1 = await safeSdk1.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse1)

Expand Down Expand Up @@ -442,7 +442,7 @@ describe('Transactions execution', () => {
const tx = await safeSdk1.createTransaction({ safeTransactionData })

// Signature: on-chain
const txHash = await safeSdk1.getHash(tx)
const txHash = await safeSdk1.getTransactionHash(tx)
const txResponse1 = await safeSdk1.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse1)

Expand Down Expand Up @@ -508,7 +508,7 @@ describe('Transactions execution', () => {
const tx = await safeSdk1.createTransaction({ safeTransactionData })

// Signature: on-chain
const txHash = await safeSdk1.getHash(tx)
const txHash = await safeSdk1.getTransactionHash(tx)
const txResponse1 = await safeSdk1.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse1)

Expand Down Expand Up @@ -560,7 +560,7 @@ describe('Transactions execution', () => {
}
const tx = await safeSdk1.createTransaction({ safeTransactionData })
const signedTx = await safeSdk1.signTransaction(tx)
const txHash = await safeSdk2.getHash(tx)
const txHash = await safeSdk2.getTransactionHash(tx)
const txResponse1 = await safeSdk2.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse1)
const txResponse2 = await safeSdk3.executeTransaction(signedTx)
Expand Down Expand Up @@ -840,7 +840,7 @@ describe('Transactions execution', () => {
]
const multiSendTx = await safeSdk1.createTransaction({ safeTransactionData })
const signedMultiSendTx = await safeSdk1.signTransaction(multiSendTx)
const txHash = await safeSdk2.getHash(multiSendTx)
const txHash = await safeSdk2.getTransactionHash(multiSendTx)
const txResponse1 = await safeSdk2.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse1)
const txResponse2 = await safeSdk3.executeTransaction(signedMultiSendTx)
Expand Down Expand Up @@ -897,7 +897,7 @@ describe('Transactions execution', () => {
]
const multiSendTx = await safeSdk1.createTransaction({ safeTransactionData })
const signedMultiSendTx = await safeSdk1.signTransaction(multiSendTx)
const txHash = await safeSdk2.getHash(multiSendTx)
const txHash = await safeSdk2.getTransactionHash(multiSendTx)
const txResponse1 = await safeSdk2.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse1)
const txResponse2 = await safeSdk3.executeTransaction(signedMultiSendTx)
Expand Down
2 changes: 1 addition & 1 deletion packages/protocol-kit/tests/e2e/offChainSignatures.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('Off-chain signatures', () => {
data: '0x'
}
const tx = await safeSdk.createTransaction({ safeTransactionData })
const txHash = await safeSdk.getHash(tx)
const txHash = await safeSdk.getTransactionHash(tx)
const signature = await safeSdk.signHash(txHash)
chai.expect(signature.staticPart().length).to.be.eq(132)
})
Expand Down
8 changes: 4 additions & 4 deletions packages/protocol-kit/tests/e2e/onChainSignatures.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('On-chain signatures', () => {
data: '0x'
}
const tx = await safeSdk1.createTransaction({ safeTransactionData })
const hash = await safeSdk1.getHash(tx)
const hash = await safeSdk1.getTransactionHash(tx)
await chai
.expect(safeSdk1.approveTransactionHash(hash))
.to.be.rejectedWith('Transaction hashes can only be approved by Safe owners')
Expand All @@ -89,7 +89,7 @@ describe('On-chain signatures', () => {
data: '0x'
}
const tx = await safeSdk1.createTransaction({ safeTransactionData })
const txHash = await safeSdk1.getHash(tx)
const txHash = await safeSdk1.getTransactionHash(tx)
const txResponse = await safeSdk1.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse)
chai.expect(await safe.approvedHashes(account1.address, txHash)).to.be.equal(1n)
Expand All @@ -111,7 +111,7 @@ describe('On-chain signatures', () => {
data: '0x'
}
const tx = await safeSdk1.createTransaction({ safeTransactionData })
const txHash = await safeSdk1.getHash(tx)
const txHash = await safeSdk1.getTransactionHash(tx)
chai.expect(await safe.approvedHashes(account1.address, txHash)).to.be.equal(0n)
const txResponse1 = await safeSdk1.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse1)
Expand Down Expand Up @@ -155,7 +155,7 @@ describe('On-chain signatures', () => {
data: '0x'
}
const tx = await safeSdk1.createTransaction({ safeTransactionData })
const txHash = await safeSdk1.getHash(tx)
const txHash = await safeSdk1.getTransactionHash(tx)
const ownersWhoApproved0 = await safeSdk1.getOwnersWhoApprovedTx(txHash)
chai.expect(ownersWhoApproved0.length).to.be.eq(0)
const txResponse1 = await safeSdk1.approveTransactionHash(txHash)
Expand Down
2 changes: 1 addition & 1 deletion playground/api-kit/propose-transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ async function main() {
const safeTransaction = await safe.createTransaction({ safeTransactionData })

const senderAddress = await signer.getAddress()
const safeTxHash = await safe.getHash(safeTransaction)
const safeTxHash = await safe.getTransactionHash(safeTransaction)
const signature = await safe.signHash(safeTxHash)

// Propose transaction to the service
Expand Down
Loading

0 comments on commit 144ec46

Please sign in to comment.