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

Feature/version 3.0 #8

Merged
merged 7 commits into from
Oct 15, 2024
Merged

Feature/version 3.0 #8

merged 7 commits into from
Oct 15, 2024

Conversation

borislav-itskov
Copy link
Owner

@borislav-itskov borislav-itskov commented Sep 18, 2023

Change log:

  • remove multiSigSign
  • rename multiSigSignHash to multiSigSign
  • do the same with verify and verifyHash

Problem: we have a lot of methods that do kind of the same but with small differences. Combine those methods into one.

@LorhanSohaky
Copy link
Contributor

Why do you want to remove musig?

In my use case it is very important

@borislav-itskov
Copy link
Owner Author

@LorhanSohaky Currently, we have two multisig methods:

multiSigSign()
multiSigSignHash()

And on top of that, multiSigSign has an additional parameter hashFn: Function|null = null. We have 3 ways to make a signature and in my opinion, this is very confusing.
I propose we omit the multiSigSign function and rename the multiSigSignHash function to multiSigSign in version 3.0. This way, we will have 1 function for making a multisigner signature and it would be more straightforward.
What do you think? I'm open to suggestions

@LorhanSohaky
Copy link
Contributor

LorhanSohaky commented Sep 18, 2023

@borislav-itskov Okay, I had only read the changelog and the last time I saw the project did not exists the hashFn parameter.

My only suggestion is to have a hash function just for convenience

export const hash = (msg: string) => ethers.utils.solidityKeccak256(['string'], [msg])

@borislav-itskov borislav-itskov marked this pull request as ready for review October 15, 2024 08:43
@borislav-itskov borislav-itskov self-assigned this Oct 15, 2024
@borislav-itskov borislav-itskov added the enhancement New feature or request label Oct 15, 2024
@borislav-itskov borislav-itskov merged commit 356c1a9 into main Oct 15, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants