Skip to content

Commit

Permalink
Rename getTransactionHash and signTransactionHash
Browse files Browse the repository at this point in the history
  • Loading branch information
yagopv committed Oct 10, 2023
1 parent bcc2354 commit eb92a4a
Show file tree
Hide file tree
Showing 12 changed files with 68 additions and 70 deletions.
10 changes: 5 additions & 5 deletions guides/integrating-the-safe-core-sdk.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,14 +207,14 @@ 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 `getTransactionHash` from the Protocol Kit.
- `safeTxHash`: The Safe transaction hash, calculated by calling the method `getHash` 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.getTransactionHash(safeTransaction)
const senderSignature = await safeSdk.signTransactionHash(safeTxHash)
const safeTxHash = await safeSdk.getHash(safeTransaction)
const senderSignature = await safeSdk.signHash(safeTxHash)
await safeService.proposeTransaction({
safeAddress,
safeTransactionData: safeTransaction.data,
Expand Down Expand Up @@ -288,13 +288,13 @@ type SafeMultisigTransactionResponse = {

## <a name="confirm-transaction">7. Confirm/reject the transaction</a>

The owners of the Safe can now sign the transaction obtained from the Safe Transaction Service by calling the method `signTransactionHash` from the Protocol Kit to generate the signature and by calling the method `confirmTransaction` from the Safe API Kit to add the signature to the service.
The owners of the Safe can now sign the transaction obtained from the Safe Transaction Service by calling the method `signHash` from the Protocol Kit to generate the signature and by calling the method `confirmTransaction` from the Safe API Kit to add the signature to the service.

```js
// transaction: SafeMultisigTransactionResponse

const hash = transaction.safeTxHash
let signature = await safeSdk.signTransactionHash(hash)
let signature = await safeSdk.signHash(hash)
await safeService.confirmTransaction(hash, signature.data)
```

Expand Down
4 changes: 2 additions & 2 deletions packages/api-kit/tests/endpoint/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ describe('Endpoint tests', () => {
const signerAddress = await signer.getAddress()
const safeSdk = await Safe.create({ ethAdapter, safeAddress })
const safeTransaction = await safeSdk.createTransaction({ safeTransactionData })
const senderSignature = await safeSdk.signTransactionHash(safeTxHash)
const senderSignature = await safeSdk.signHash(safeTxHash)
await chai
.expect(
safeApiKit.proposeTransaction({
Expand Down Expand Up @@ -407,7 +407,7 @@ describe('Endpoint tests', () => {
const signerAddress = await signer.getAddress()
const safeSdk = await Safe.create({ ethAdapter, safeAddress })
const safeTransaction = await safeSdk.createTransaction({ safeTransactionData })
const senderSignature = await safeSdk.signTransactionHash(safeTxHash)
const senderSignature = await safeSdk.signHash(safeTxHash)
await chai
.expect(
safeApiKit.proposeTransaction({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ describe('SafeMoneriumClient', () => {
data: txData
})

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

jest.spyOn(SafeApiKit.prototype, 'getTransaction').mockResolvedValueOnce({
confirmationsRequired: 1,
Expand Down
4 changes: 2 additions & 2 deletions packages/onramp-kit/src/packs/monerium/SafeMoneriumClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ export class SafeMoneriumClient extends MoneriumClient {
}
})

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

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

const chainId = await this.#safeSdk.getChainId()

Expand Down
16 changes: 8 additions & 8 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.getTransactionHash(safeTransaction)
const txHash = await safeSdk2.getHash(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)
```
### getTransactionHash
### getHash
Returns the transaction hash of a Safe transaction.
Expand All @@ -642,10 +642,10 @@ const safeTransactionData: SafeTransactionDataPartial = {
// ...
}
const safeTransaction = await safeSdk.createTransaction({ safeTransactionData })
const txHash = await safeSdk.getTransactionHash(safeTransaction)
const txHash = await safeSdk.getHash(safeTransaction)
```
### signTransactionHash
### signHash
Signs a hash using the current owner account.
Expand All @@ -654,8 +654,8 @@ const safeTransactionData: SafeTransactionDataPartial = {
// ...
}
const safeTransaction = await safeSdk.createTransaction({ safeTransactionData })
const txHash = await safeSdk.getTransactionHash(safeTransaction)
const signature = await safeSdk.signTransactionHash(txHash)
const txHash = await safeSdk.getHash(safeTransaction)
const signature = await safeSdk.signHash(txHash)
```
### signTypedData
Expand Down Expand Up @@ -701,7 +701,7 @@ const safeTransactionData: SafeTransactionDataPartial = {
// ...
}
const safeTransaction = await safeSdk.createTransaction({ safeTransactionData })
const txHash = await safeSdk.getTransactionHash(safeTransaction)
const txHash = await safeSdk.getHash(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.getTransactionHash(safeTransaction)
const txHash = await safeSdk.getHash(safeTransaction)
const ownerAddresses = await safeSdk.getOwnersWhoApprovedTx(txHash)
```
Expand Down
18 changes: 12 additions & 6 deletions packages/protocol-kit/src/Safe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
MetaTransactionData,
Transaction
} from '@safe-global/safe-core-sdk-types'
import { soliditySha3, utf8ToHex } from 'web3-utils'
import {
PREDETERMINED_SALT_NONCE,
encodeSetupCallData,
Expand Down Expand Up @@ -492,11 +493,16 @@ class Safe {
* @param safeTransaction - The Safe transaction
* @returns The transaction hash of the Safe transaction
*/
async getTransactionHash(safeTransaction: SafeTransaction): Promise<string> {
async getHash(txOrMessage: SafeTransaction | string): Promise<string> {
if (!this.#contractManager.safeContract) {
throw new Error('Safe is not deployed')
}
const safeTransactionData = safeTransaction.data

if (typeof txOrMessage === 'string') {
return soliditySha3(utf8ToHex(txOrMessage)) || ''
}

const safeTransactionData = txOrMessage.data
const txHash = await this.#contractManager.safeContract.getTransactionHash(safeTransactionData)
return txHash
}
Expand Down Expand Up @@ -584,8 +590,8 @@ 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.getTransactionHash(transaction)
signature = await this.signTransactionHash(txHash)
const txHash = await this.getHash(transaction)
signature = await this.signHash(txHash)
}

const signedSafeTransaction = await this.createTransaction({
Expand Down Expand Up @@ -942,7 +948,7 @@ class Safe {

const signedSafeTransaction = await this.copyTransaction(transaction)

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

const signedSafeTransaction = await this.copyTransaction(transaction)

const txHash = await this.getTransactionHash(signedSafeTransaction)
const txHash = await this.getHash(signedSafeTransaction)
const ownersWhoApprovedTx = await this.getOwnersWhoApprovedTx(txHash)
for (const owner of ownersWhoApprovedTx) {
signedSafeTransaction.addSignature(generatePreValidatedSignature(owner))
Expand Down
44 changes: 18 additions & 26 deletions packages/protocol-kit/tests/e2e/eip1271.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ import { getAccounts } from './utils/setupTestNetwork'
import { waitSafeTxReceipt } from './utils/transactions'
import { itif } from './utils/helpers'
import { BigNumber, ethers } from 'ethers'
import { EthSafeSignature, buildSignature } from '@safe-global/protocol-kit/utils'
import { soliditySha3, utf8ToHex } from 'web3-utils'
import { buildSignature } from '@safe-global/protocol-kit/utils'

chai.use(chaiAsPromised)

Expand All @@ -28,18 +27,14 @@ export const calculateSafeMessageHash = (
)
}

const hashMessage = (message: string): string => {
return soliditySha3(utf8ToHex(message)) || '0x'
}

export const EIP712_SAFE_MESSAGE_TYPE = {
// "SafeMessage(bytes message)"
SafeMessage: [{ type: 'bytes', name: 'message' }]
}

const MESSAGE = 'I am the owner of this Safe account'

describe.only('EIP1271', () => {
describe('EIP1271', () => {
describe('Using a 2/3 Safe in the context of the EIP1271', async () => {
const setupTests = deployments.createFixture(async ({ deployments }) => {
await deployments.fixture()
Expand Down Expand Up @@ -113,7 +108,9 @@ describe.only('EIP1271', () => {
customContractAbi: customContract.signMessageLibAbi
})

const txData = signMessageLibContract.encode('signMessage', [hashMessage(MESSAGE)])
const messageHash = await safeSdk1.getHash(MESSAGE)

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

const safeTransactionData: SafeTransactionDataPartial = {
to: customContract.signMessageLibAddress,
Expand All @@ -129,10 +126,10 @@ describe.only('EIP1271', () => {

await waitSafeTxReceipt(execResponse)

const validatedResponse1 = await safeSdk1.isValidSignature(hashMessage(MESSAGE))
const validatedResponse1 = await safeSdk1.isValidSignature(messageHash)
chai.expect(validatedResponse1).to.be.true

const validatedResponse2 = await safeSdk1.isValidSignature(hashMessage(MESSAGE), '0x')
const validatedResponse2 = await safeSdk1.isValidSignature(messageHash, '0x')
chai.expect(validatedResponse2).to.be.true
}
)
Expand All @@ -141,7 +138,7 @@ describe.only('EIP1271', () => {
const { safeSdk1, safeSdk2 } = await setupTests()

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

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

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

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

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

// Sign the Safe message with the owners
Expand All @@ -218,7 +215,7 @@ 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(hashMessage(MESSAGE), '0x')
const response = await safeSdk1.isValidSignature(await safeSdk1.getHash(MESSAGE), '0x')

chai.expect(response).to.be.false
})
Expand All @@ -229,11 +226,12 @@ describe.only('EIP1271', () => {
const { safe, safeSdk1 } = await setupTests()

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

chai
.expect(safeMessageHash)
.to.be.eq(calculateSafeMessageHash(safe.address, hashMessage(MESSAGE), chainId))
.to.be.eq(calculateSafeMessageHash(safe.address, messageHash, chainId))
}
)

Expand All @@ -258,21 +256,15 @@ describe.only('EIP1271', () => {

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

const messageHash = await safeSdk1.getTransactionHash(tx)
const txHash = await safeSdk1.getHash(tx)

const signature1 = await safeSdk1.signHash(await safeSdk1.getSafeMessageHash(messageHash))
const signature1 = await safeSdk1.signHash(await safeSdk1.getSafeMessageHash(txHash))

const signature2 = await safeSdk3.signHash(
await safeSdk3.getSafeMessageHash(messageHash),
true
)
const signature2 = await safeSdk3.signHash(await safeSdk3.getSafeMessageHash(txHash), true)
console.log('OWNER 1: ', signature1.signer)
console.log('OWNER 2: ', signature2.signer)

const isValidSignature = await safeSdk1.isValidSignature(messageHash, [
signature1,
signature2
])
const isValidSignature = await safeSdk1.isValidSignature(txHash, [signature1, signature2])
console.log('IS VALID SIGNATURE: ', isValidSignature)
chai.expect(isValidSignature).to.be.true

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 @@ -144,7 +144,7 @@ describe('Transactions execution', () => {
}
const tx = await safeSdk1.createTransaction({ safeTransactionData })
const signedTx = await safeSdk1.signTransaction(tx)
const txHash = await safeSdk2.getTransactionHash(tx)
const txHash = await safeSdk2.getHash(tx)
const txResponse = await safeSdk2.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse)
await chai
Expand Down Expand Up @@ -321,7 +321,7 @@ describe('Transactions execution', () => {
const tx = await safeSdk1.createTransaction({ safeTransactionData })

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

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

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

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

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

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

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

Expand Down Expand Up @@ -557,7 +557,7 @@ describe('Transactions execution', () => {
}
const tx = await safeSdk1.createTransaction({ safeTransactionData })
const signedTx = await safeSdk1.signTransaction(tx)
const txHash = await safeSdk2.getTransactionHash(tx)
const txHash = await safeSdk2.getHash(tx)
const txResponse1 = await safeSdk2.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse1)
const txResponse2 = await safeSdk3.executeTransaction(signedTx)
Expand Down Expand Up @@ -835,7 +835,7 @@ describe('Transactions execution', () => {
]
const multiSendTx = await safeSdk1.createTransaction({ safeTransactionData })
const signedMultiSendTx = await safeSdk1.signTransaction(multiSendTx)
const txHash = await safeSdk2.getTransactionHash(multiSendTx)
const txHash = await safeSdk2.getHash(multiSendTx)
const txResponse1 = await safeSdk2.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse1)
const txResponse2 = await safeSdk3.executeTransaction(signedMultiSendTx)
Expand Down Expand Up @@ -892,7 +892,7 @@ describe('Transactions execution', () => {
]
const multiSendTx = await safeSdk1.createTransaction({ safeTransactionData })
const signedMultiSendTx = await safeSdk1.signTransaction(multiSendTx)
const txHash = await safeSdk2.getTransactionHash(multiSendTx)
const txHash = await safeSdk2.getHash(multiSendTx)
const txResponse1 = await safeSdk2.approveTransactionHash(txHash)
await waitSafeTxReceipt(txResponse1)
const txResponse2 = await safeSdk3.executeTransaction(signedMultiSendTx)
Expand Down
Loading

0 comments on commit eb92a4a

Please sign in to comment.