Finschia / finschia-sdk

A framework for building blockchains based Finschia Mainnet that is forked from cosmos-sdk
Apache License 2.0
61 stars 31 forks source link

fix: prevent signing from wrong key in multisig (backport #1319) #1324

Closed mergify[bot] closed 8 months ago

mergify[bot] commented 8 months ago

Description

closes: #XXXX

When signing an multisig tx, you are required to provide the multisig address (--multisig) and the key you are signing with (--from), but there's no check that the key is actually part of the multisig. This makes it very easy to accidentally sign with the wrong key and only figure it out when you try to broadcast the invalid tx that includes a signature from a key thats not in the multisig.

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Checklist:

mergify[bot] commented 8 months ago

Cherry-pick of c051dcc91d032648297656ebf3d9e00613d0aa58 has failed:

On branch mergify/bp/release/v0.48.x/pr-1319
Your branch is up to date with 'origin/release/v0.48.x'.

You are currently cherry-picking commit c051dcc91.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
    modified:   x/auth/client/cli/tx_multisign.go
    modified:   x/auth/client/cli/tx_sign.go
    modified:   x/auth/client/testutil/suite.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
    both modified:   CHANGELOG.md

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

codecov[bot] commented 8 months ago

Codecov Report

Attention: Patch coverage is 34.04255% with 31 lines in your changes are missing coverage. Please review.

Project coverage is 69.76%. Comparing base (1f8e902) to head (6e8c3a6). Report is 10 commits behind head on release/v0.48.x.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/Finschia/finschia-sdk/pull/1324/graphs/tree.svg?width=650&height=150&src=pr&token=m16qfzIPO7&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia)](https://app.codecov.io/gh/Finschia/finschia-sdk/pull/1324?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia) ```diff @@ Coverage Diff @@ ## release/v0.48.x #1324 +/- ## =================================================== - Coverage 69.78% 69.76% -0.02% =================================================== Files 646 646 Lines 67507 67573 +66 =================================================== + Hits 47109 47142 +33 - Misses 18212 18242 +30 - Partials 2186 2189 +3 ``` | [Files](https://app.codecov.io/gh/Finschia/finschia-sdk/pull/1324?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia) | Coverage Δ | | |---|---|---| | [x/auth/client/testutil/suite.go](https://app.codecov.io/gh/Finschia/finschia-sdk/pull/1324?src=pr&el=tree&filepath=x%2Fauth%2Fclient%2Ftestutil%2Fsuite.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia#diff-eC9hdXRoL2NsaWVudC90ZXN0dXRpbC9zdWl0ZS5nbw==) | `96.94% <100.00%> (+0.03%)` | :arrow_up: | | [x/auth/client/cli/tx\_multisign.go](https://app.codecov.io/gh/Finschia/finschia-sdk/pull/1324?src=pr&el=tree&filepath=x%2Fauth%2Fclient%2Fcli%2Ftx_multisign.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia#diff-eC9hdXRoL2NsaWVudC9jbGkvdHhfbXVsdGlzaWduLmdv) | `0.00% <0.00%> (ø)` | | | [x/auth/client/cli/tx\_sign.go](https://app.codecov.io/gh/Finschia/finschia-sdk/pull/1324?src=pr&el=tree&filepath=x%2Fauth%2Fclient%2Fcli%2Ftx_sign.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia#diff-eC9hdXRoL2NsaWVudC9jbGkvdHhfc2lnbi5nbw==) | `0.00% <0.00%> (ø)` | | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/Finschia/finschia-sdk/pull/1324/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia)