-
Notifications
You must be signed in to change notification settings - Fork 47
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
always use eth_personal_sign #278
Comments
Example of where we use .sign instead of personal.sign loom-js/src/contracts/address-mapper.ts Line 49 in 02ef5d5
|
@lukezhangstudio that function is passing an interface for async addIdentityMappingAsync(
from: Address,
to: Address,
ethersSigner: IEthereumSigner
): Promise<Uint8Array | void> {
const mappingIdentityRequest = new AddressMapperAddIdentityMappingRequest()
mappingIdentityRequest.setFrom(from.MarshalPB())
mappingIdentityRequest.setTo(to.MarshalPB())
const hash = soliditySha3(
{
type: 'address',
value: from.local.toString().slice(2)
},
{
type: 'address',
value: to.local.toString().slice(2)
}
)
const sign = await ethersSigner.signAsync(hash)
mappingIdentityRequest.setSignature(sign)
return this.callAsync<void>('AddIdentityMapping', mappingIdentityRequest)
} loom-js/src/solidity-helpers.ts Line 47 in 877edfc
Instead of loom-js/src/solidity-helpers.ts Line 79 in 877edfc
|
@lukezhangstudio could you please confirm if my comment above makes sense?
|
Because of the way Ethers.js implements a workaround for metamask, we have been effectively using personal.sign. Let's just switch to using personal_sign explicitly in case someone imports addressmapper without using dposuser.
Our own workaround:
loom-js/src/solidity-helpers.ts
Line 24 in a128250
The text was updated successfully, but these errors were encountered: