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

Add basic multisig Account #21

Open
martriay opened this issue Oct 23, 2021 · 6 comments · May be fixed by #924
Open

Add basic multisig Account #21

martriay opened this issue Oct 23, 2021 · 6 comments · May be fixed by #924

Comments

@martriay
Copy link
Contributor

This should be implemented in the is_valid_signature function.

This is how it looks for single signature accounts:

@view
func is_valid_signature{
        storage_ptr: Storage*,
        pedersen_ptr: HashBuiltin*,
        ecdsa_ptr: SignatureBuiltin*,
        syscall_ptr: felt*,
        range_check_ptr
    } (
        hash: felt,
        signature_len: felt,
        signature: felt*
    ) -> ():
    assert_initialized()
    let (_public_key) = public_key.read()
    # This interface expects a signature pointer and length to make
    # no assumption about signature validation schemes.
    # But this implementation does, and it expects a (sig_r, sig_s) pair.
    let sig_r = signature[0]
    let sig_s = signature[1]

    verify_ecdsa_signature(
        message=hash,
        public_key=_public_key,
        signature_r=sig_r,
        signature_s=sig_s)

    return ()
end
@martriay martriay added the enhancement New feature or request label Oct 23, 2021
@martriay martriay added feature and removed enhancement New feature or request labels Oct 24, 2021
@martriay
Copy link
Contributor Author

martriay commented Oct 26, 2021

One possible implementation can be found in argent's account contract.

It performs each check separately, which is basically the same logic but for a privileged signer.

validate_signer_signature(message_hash, signatures, signatures_len, 0)
validate_guardian_signature(message_hash, signatures, signatures_len, 1)

@TAdev0
Copy link
Contributor

TAdev0 commented Jan 31, 2024

@martriay may i work on this issue?

@martriay
Copy link
Contributor Author

Really appreciate the energy! But let's focus on closing the other PRs first :)

@TAdev0
Copy link
Contributor

TAdev0 commented Feb 22, 2024

@martriay hi, tell me when we're good to start working on this feature, if no one else is working on it yet :)

@martriay
Copy link
Contributor Author

sure! although i believe this one is a bit heavy on design, so i would focus on that way before opening any full implementation PR -- of course it makes sense to fiddle around with code while designing, just a warning on not overdoing it like tests or full implementations if we notice it's a bad design path

@martriay
Copy link
Contributor Author

some challenges i foresee here is having a different storage struct due to having multiple and not a single owner as our current designs, and how to manage the code duplication if we were to have two different components, etc

@TAdev0 TAdev0 linked a pull request Feb 23, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📋 Backlog
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants