Conversation
# Description Closes: #1818 ## Criteria for change/create btcstaking library functions - don't introduce api breaking changes to exported functions (i.e., function start with upper case) possibly used in external programs, including [btc-staker](https://github.com/babylonlabs-io/btc-staker), [vigilante](https://github.com/babylonlabs-io/vigilante/blob/v0.24.0-rc.4/btcstaking-tracker/README.md), [covenant-emulator](https://github.com/babylonlabs-io/covenant-emulator), etc. - enhance code reusability by modifying internal functions (i.e., function start with lower case) ## functions list that are newly created or modified ```go // btcstaking/identifiable_staking.go BuildV0IdentifiableMultisigStakingOutputs BuildV0IdentifiableMultisigStakingOutputsAndTx ParseV0MultisigStakingTx // btcstaking/script_utils.go assembleMultiSigScript buildMultiSigScript // btcstaking/staking.go validateSlashingTx CheckSlashingTxMatchingFundingTxMultisig VerifyTransactionMultiSigWithOutput verifyTaprootScriptSpendSignature BuildMultisigSlashingTxFromStakingTxStrict // btcstaking/types.go checkForDuplicateKeys newBabylonScriptPaths BuildMultisigStakingInfo BuildMultisigUnbondingInfo ``` --- ## Author Checklist **All** items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues. I have... - [ ] tackled an existing issue or discussed with a team member - [ ] left instructions on how to review the changes - [ ] targeted the `main` branch ## Reviewers Checklist **All** items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items. I have... - [ ] added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [ ] confirmed all author checklist items have been addressed - [ ] confirmed that this PR does not change production code - [ ] reviewed content - [ ] tested instructions (if applicable) - [ ] confirmed all CI checks have passed
# Description Closes: #1819 ### Criteria for modification - don't introduce api breaking changes to existing txs, queries services ### Changes - `CreateBTCDelegation` - `BTCStakeExpand` - `BTCDelegation` and `BTCDelegations` Note: upgrade handler will be handled in other PR, will rebase this PR once #1822 is merged into feat/staker-multi-sig branch --- ## Author Checklist **All** items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues. I have... - [ ] tackled an existing issue or discussed with a team member - [ ] left instructions on how to review the changes - [ ] targeted the `main` branch ## Reviewers Checklist **All** items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items. I have... - [ ] added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [ ] confirmed all author checklist items have been addressed - [ ] confirmed that this PR does not change production code - [ ] reviewed content - [ ] tested instructions (if applicable) - [ ] confirmed all CI checks have passed
# Description Closes: #1820 --- ## Author Checklist **All** items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues. I have... - [ ] tackled an existing issue or discussed with a team member - [ ] left instructions on how to review the changes - [ ] targeted the `main` branch ## Reviewers Checklist **All** items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items. I have... - [ ] added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [ ] confirmed all author checklist items have been addressed - [ ] confirmed that this PR does not change production code - [ ] reviewed content - [ ] tested instructions (if applicable) - [ ] confirmed all CI checks have passed
# Description Closes: #1835 --- ## Author Checklist **All** items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues. I have... - [ ] tackled an existing issue or discussed with a team member - [ ] left instructions on how to review the changes - [ ] targeted the `main` branch ## Reviewers Checklist **All** items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items. I have... - [ ] added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [ ] confirmed all author checklist items have been addressed - [ ] confirmed that this PR does not change production code - [ ] reviewed content - [ ] tested instructions (if applicable) - [ ] confirmed all CI checks have passed
# Description Closes: #1840 --- ## Author Checklist **All** items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues. I have... - [x] tackled an existing issue or discussed with a team member - [ ] left instructions on how to review the changes - [ ] targeted the `main` branch ## Reviewers Checklist **All** items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items. I have... - [ ] added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [ ] confirmed all author checklist items have been addressed - [ ] confirmed that this PR does not change production code - [ ] reviewed content - [ ] tested instructions (if applicable) - [ ] confirmed all CI checks have passed
|
hey @RafilxTenfen @KonradStaniec @jhBharvest , in addition to those PRs that are listed in the description, I've added a testnet cli |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for M-of-N multisig BTC stakers by introducing new multisig delegation capabilities to the btcstaking module. The changes allow multiple stakers to jointly control a BTC delegation using threshold signatures.
- Adds
MaxStakerQuorumandMaxStakerNumparameters to control maximum M-of-N multisig configurations - Implements
AdditionalStakerInfoandMultisigInfostructures to store extra staker keys and signatures - Updates message validation to handle multisig delegations with proper quorum checks
- Adds migration logic (v1 to v2) to initialize new parameters with default values (1-of-1, equivalent to single-sig)
Reviewed Changes
Copilot reviewed 56 out of 56 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| x/btcstaking/types/validate_parsed_message_test.go | Adds test helper functions and test cases for multisig delegation validation |
| x/btcstaking/types/validate_parsed_message.go | Implements validation logic for multisig delegations with quorum checks |
| x/btcstaking/types/params.pb.go | Adds protobuf definitions for MaxStakerQuorum and MaxStakerNum |
| x/btcstaking/types/params.go | Implements validation logic for multisig parameters |
| x/btcstaking/types/params_test.go | Adds tests for multisig parameter validation |
| x/btcstaking/types/tx.pb.go | Adds protobuf definitions for MultisigInfo in delegation messages |
| x/btcstaking/types/query.pb.go | Adds AdditionalStakerInfoResponse for query responses |
| x/btcstaking/types/events.pb.go | Adds multisig staker public key fields to delegation created event |
| x/btcstaking/types/create_delegation_parser.go | Implements parsing logic for multisig info with duplicate key detection |
| x/btcstaking/keeper/btc_delegations.go | Updates delegation creation to handle multisig info |
| x/btcstaking/module.go | Registers migration from v1 to v2 and updates consensus version |
| x/btcstaking/migrations/v2/store.go | Implements migration to initialize multisig parameters |
| x/finality/keeper/power_dist_change_test.go | Updates test to include new multisig parameters |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
will re-open this PR after finishing #1861 to support vigilante |
# Description Closes: #1862 --- ## Author Checklist **All** items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues. I have... - [ ] tackled an existing issue or discussed with a team member - [ ] left instructions on how to review the changes - [ ] targeted the `main` branch ## Reviewers Checklist **All** items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items. I have... - [ ] added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [ ] confirmed all author checklist items have been addressed - [ ] confirmed that this PR does not change production code - [ ] reviewed content - [ ] tested instructions (if applicable) - [ ] confirmed all CI checks have passed
### As-Is When `AddCovenantSigs` for stake expansion sigs, it doesn't validate prev btc delegation successfully if it's a multisig btc delegation. ### To-Be https://github.com/babylonlabs-io/babylon/blob/13f784296bd8e61ea6f73e91a4dd7e9fb9e45e1b/x/btcstaking/keeper/msg_server.go#L465-L476 `GetMultisigStakingInfo` if prev btc delegation is a multisig btc delegation. ### Note Currently, investigating other potential problems in the babylon to support multisig btc delegation in btc stake expansion flow. It was found when adding stake expansion e2e test in covenant-emulator. After founding this issue, I've checked all the functions `GetStakingInfo`, `GetUnbondingInfo`, `BuildStakingInfo`, and `BuildUnbondingInfo` for any potential issue. and there's none.
### Description This PR fixes `BTCUndelegate` to properly deal with multisig btc delegation. Before this fix, `VerifySpendStakeTxStakerSig` doesn't successfully verify spend stake tx signature. This pr targets to fix it.
| @@ -13,10 +13,12 @@ import ( | |||
| // private helper to assemble multisig script | |||
| // if `withVerify` is true script will end with OP_NUMEQUALVERIFY otherwise with OP_NUMEQUAL | |||
| // SCRIPT: <Pk1> OP_CHEKCSIG <Pk2> OP_CHECKSIGADD <Pk3> OP_CHECKSIGADD ... <PkN> OP_CHECKSIGADD <threshold> OP_NUMEQUALVERIFY (or OP_NUMEQUAL) | |||
There was a problem hiding this comment.
hey @KonradStaniec do you have any idea on using OP_NUMEQUAL rather than OP_GREATERTHANOREQUAL for multisig script?
I've already implemented btc delegation multisig with OP_NUMEQUALVERIFY which exists in the current code base, but considering that OP_GREATERTHANOREQUAL can be also an option. cc. @RafilxTenfen
There was a problem hiding this comment.
Hey @canu0205, @KonradStaniec is on leave for a while.
I believe that OP_NUMEQUALVERIFY is good because:
- Enforces exact threshold
- Prevents "too many signatures"
while the OP_GREATERTHANOREQUAL we could get more signatures than needed and if we have the ability to control that I would say it is better
There was a problem hiding this comment.
yeah @RafilxTenfen, I need thorough review before saying we can control more signatures than M (M-of-N multisig). Maybe we can leave it as a backlog and then come back once everything is working correctly, including other programs (btc-staker, vigilante, covenant-emulator), and babylon.
### Description support below CLI commands to send multisig btc delegation tx: - `babylond tx btcstaking create-btc-delegation` - `babylond tx btcstaking btc-stake-expand`
document on btc multisignature stake
### Description This pr targets to improve e2ev2 multisig test coverage, which contains: - [x] complete multisig stake expansion flow (`CreateBTCDelegation`, `AddBTCDelegationInclusionProof`, `AddCovenantSigs`, `BtcStakeExpand`, `BTCUndelegate`) - [x] multisig btc del -> single sig stake expansion (should fail) - [x] single sig btc del -> multisig stake expansion (should fail) - [ ] ~~multisig btc del slashed~~ (N/A)
|
@babylon-devops @maiquanghiep could you publish latest commit (7989574) to the docker hub? cc. @RafilxTenfen |
## Description resolve conflicts on #1826 --------- Co-authored-by: RafilxTenfen <rafaeltenfen.rt@gmail.com> Co-authored-by: Cirrus Gai <greferry@gmail.com> Co-authored-by: tom <tomasguerraalda@hotmail.com> Co-authored-by: Tom <54514587+GAtom22@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Description
Closes: #1817
This feature branch will be split into few PRs:
BTCUndelegatemultisig support fix: multisigBTCUndelegate#1866other programs