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

[Bug]: CLI Signing for Nested Multisigs #20355

Open
1 task done
brandoncurtis opened this issue May 12, 2024 · 2 comments · May be fixed by #20438
Open
1 task done

[Bug]: CLI Signing for Nested Multisigs #20355

brandoncurtis opened this issue May 12, 2024 · 2 comments · May be fixed by #20438
Assignees
Labels

Comments

@brandoncurtis
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

I expect to be able to create and sign for a multisig with the following layout:
key1 => multisig1 (1 of 1)
key1, multisig1 => multisig2 (1 of 2)

After successful creation of this nested multisig, it does not appear to be possible to use the CLI to sign a transaction for multisig2 using multisig1:

$ simd tx sign tx-from-multisig2.json --from key1 --multisig multisig2
Error: tx intended signer does not match the given signer: key1
$ simd tx sign tx-from-multisig2.json --from key1 --multisig multisig1
Error: tx intended signer does not match the given signer: key1
$ simd tx sign tx-from-multisig2.json --from multisig1 --multisig multisig2
Error: cannot sign with offline keys

Cosmos SDK Version

0.47.x

How to reproduce?

No response

@facundomedica
Copy link
Member

Found 2 things:

  1. The tx signer checks are too naive

Whenever we check if a signer is in the tx we just check if the signer's address is included. When a multisig is involved, we should go through the multisig members and see if the signer is there.

  1. We don't have a CLI to wrap signatures under a multisig signature

In your example key1's signature will be exactly the same when participating in the multisig and by itself, the difference will be in how we wrap it.

individual sig:

{
    "signatures": [
        {
            "public_key": {
                "@type": "/cosmos.crypto.secp256k1.PubKey",
                "key": "AwXEjIy2GGcOIssc0BQVdrCu1AgakSaKbd1RMOhijJFQ"
            },
            "data": {
                "single": {
                    "mode": "SIGN_MODE_LEGACY_AMINO_JSON",
                    "signature": "YsRnydRcjSom8UwV2R6zFIBpIUc3sa8nD8367sq8vTMn+vXI3+9h3UbDak2NA0zbHG58oowKtiYW27/M62xFvA=="
                }
            },
            "sequence": "0"
        }
    ]
}

multisig sig:

{
    "signatures": [
        {
            "public_key": {
                "@type": "/cosmos.crypto.multisig.LegacyAminoPubKey",
                "threshold": 1,
                "public_keys": [
                    {
                        "@type": "/cosmos.crypto.secp256k1.PubKey",
                        "key": "AwXEjIy2GGcOIssc0BQVdrCu1AgakSaKbd1RMOhijJFQ"
                    }
                ]
            },
            "data": {
                "multi": {
                    "bitarray": {
                        "extra_bits_stored": 1,
                        "elems": "gA=="
                    },
                    "signatures": [
                        {
                            "single": {
                                "mode": "SIGN_MODE_LEGACY_AMINO_JSON",
                                "signature": "YsRnydRcjSom8UwV2R6zFIBpIUc3sa8nD8367sq8vTMn+vXI3+9h3UbDak2NA0zbHG58oowKtiYW27/M62xFvA=="
                            }
                        }
                    ]
                }
            },
            "sequence": "0"
        }
    ]
}

So we most likely need another command that does this wrapping/merging (which in this case I did manually 😆).

@brandoncurtis
Copy link
Author

Yep, that was my takeaway! How'd you do it manually?

@facundomedica facundomedica linked a pull request May 22, 2024 that will close this issue
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 📋 Backlog
Development

Successfully merging a pull request may close this issue.

2 participants