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

Closed
1 task done
brandoncurtis opened this issue May 12, 2024 · 2 comments · Fixed by #20438
Closed
1 task done

[Bug]: CLI Signing for Nested Multisigs #20355

brandoncurtis opened this issue May 12, 2024 · 2 comments · 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?

@tac0turtle tac0turtle moved this from 📋 Backlog to 👀 Waiting / In review in Cosmos-SDK Jun 3, 2024
@github-project-automation github-project-automation bot moved this from 👀 Waiting / In review to 🥳 Done in Cosmos-SDK Jun 14, 2024
@tac0turtle tac0turtle removed this from Cosmos-SDK Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants