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

Closed
martriay opened this issue Oct 23, 2021 · 6 comments · Fixed by #1193
Closed

Add basic multisig Account #21

martriay opened this issue Oct 23, 2021 · 6 comments · Fixed by #1193

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

@andrew-fleming andrew-fleming added this to the 4. later milestone Jun 15, 2024
@andrew-fleming andrew-fleming modified the milestones: 4. later, 3. after Aug 9, 2024
@ericnordelo ericnordelo modified the milestones: 1. current, 3. after Sep 25, 2024
@ggonzalez94 ggonzalez94 modified the milestones: 3. after, 2. next Oct 9, 2024
@andrew-fleming andrew-fleming modified the milestones: 2. next, 1. current Oct 18, 2024
@immrsd immrsd mentioned this issue Oct 25, 2024
4 tasks
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Resolved in Contracts for Cairo Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Status: No status
Development

Successfully merging a pull request may close this issue.

6 participants