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.

Sign up for GitHub

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

fix: incorrect data merkleRoot length #262

Merged
merged 1 commit into from
Aug 24, 2023
Merged

fix: incorrect data merkleRoot length #262

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions packages/modules/src/SessionKeyManagerModule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Signer, ethers } from 'ethers'
import MerkleTree from 'merkletreejs'
import { getUserOpHash, NODE_CLIENT_URL } from '@biconomy/common'
import { hexConcat, arrayify, keccak256, hexZeroPad, defaultAbiCoder } from 'ethers/lib/utils'
import { hexConcat, arrayify, hexZeroPad, defaultAbiCoder } from 'ethers/lib/utils'
import { keccak256 } from 'ethereumjs-util'
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to use keccak256 from ethereumjs-util in other places too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only for generating the merkle root this is required for now as the addLeaves needs buffer which we was generating and the length was incorrect

Copy link
Contributor

@livingrockrises livingrockrises Aug 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

in MultiChain Validator it would be fine? as we use keccak256 as arg in MerkleTree class

     import {
  keccak256,
  arrayify,
  defaultAbiCoder,
  hexConcat,
  hexZeroPad,
  Bytes
} from 'ethers/lib/utils'
      

     const merkleTree = new MerkleTree(leaves, keccak256, { sortPairs: true })

      const multichainSignature = await this.signer.signMessage(arrayify(merkleTree.getHexRoot()))

      Logger.log('merkle root ', merkleTree.getHexRoot())

import {
SessionKeyManagerModuleConfig,
ModuleVersion,
Expand Down Expand Up @@ -74,7 +75,7 @@ export class SessionKeyManagerModule extends BaseValidationModule {
hexZeroPad(sessionData.sessionValidationModule, 20),
sessionData.sessionKeyData
])
return Buffer.from(keccak256(leafDataHex))
return ethers.utils.keccak256(leafDataHex)
})

instance.merkleTree = new MerkleTree(existingSessionDataLeafs, keccak256, {
Expand All @@ -101,7 +102,7 @@ export class SessionKeyManagerModule extends BaseValidationModule {
hexZeroPad(leafData.sessionValidationModule, 20),
leafData.sessionKeyData
])
this.merkleTree.addLeaves([Buffer.from(keccak256(leafDataHex))])
this.merkleTree.addLeaves([ethers.utils.keccak256(leafDataHex) as unknown as Buffer])
const setMerkleRootData = sessionKeyManagerModuleInterface.encodeFunctionData('setMerkleRoot', [
this.merkleTree.getHexRoot()
])
Expand Down