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.

Sign up for GitHub

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

fix: fixing witness construction of slashing tx #193

Merged
merged 3 commits into from
Oct 14, 2024
Merged

fix: fixing witness construction of slashing tx #193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b82ba45
init
SebastianElvis Oct 14, 2024
b55faa4
changelog
SebastianElvis Oct 14, 2024
343eb84
fix
SebastianElvis Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

### Bug fixes

* [#193](https://github.com/babylonlabs-io/babylon/pull/193) Fix witness construction of slashing tx
* [#154](https://github.com/babylonlabs-io/babylon/pull/154) Fix "edit-finality-provider" cmd argument index

### Improvements
Expand Down
2 changes: 2 additions & 0 deletions x/btcstaking/types/btc_delegation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@ func (d *BTCDelegation) BuildSlashingTxWithWitness(bsParams *Params, btcNet *cha
d.StakingOutputIdx,
d.DelegatorSig,
covAdaptorSigs,
bsParams.CovenantQuorum,
slashingSpendInfo,
)
if err != nil {
Expand Down Expand Up @@ -433,6 +434,7 @@ func (d *BTCDelegation) BuildUnbondingSlashingTxWithWitness(bsParams *Params, bt
0,
d.BtcUndelegation.DelegatorSlashingSig,
covAdaptorSigs,
bsParams.CovenantQuorum,
slashingSpendInfo,
)
if err != nil {
Expand Down
3 changes: 1 addition & 2 deletions x/btcstaking/types/btc_delegation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ func FuzzBTCDelegation_SlashingTx(f *testing.F) {
slashingChangeLockTime := unbondingTime

// only the quorum of signers provided the signatures
covenantSigners := covenantSKs[:covenantQuorum]

covenantSigners := covenantSKs
// construct the BTC delegation with everything
btcDel, err := datagen.GenRandomBTCDelegation(
r,
Expand Down
10 changes: 10 additions & 0 deletions x/btcstaking/types/btc_slashing_tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ func (tx *BTCSlashingTx) BuildSlashingTxWithWitness(
outputIdx uint32,
delegatorSig *bbn.BIP340Signature,
covenantSigs []*asig.AdaptorSignature,
covenantQuorum uint32,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can return error early from this function i.e:

if len(covenantSigs) < covenantQuorum {
  return nil, fmt.Errorf("not enough signatures")
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! As covenantSigs can contain nil (in order to have the length of covenant committee size), I added the below logic below L303

	// ensure the number of covenant signatures is at least the quorum number
	if numSigs < covenantQuorum {
		return nil, fmt.Errorf("not enough covenant signatures to reach quorum")
	}

does this make sense?

slashingPathSpendInfo *btcstaking.SpendInfo,
) (*wire.MsgTx, error) {
/*
Expand All @@ -288,12 +289,21 @@ func (tx *BTCSlashingTx) BuildSlashingTxWithWitness(
}
// decrypt each covenant adaptor signature to Schnorr signature
covSigs := make([]*schnorr.Signature, len(covenantSigs))
numSigs := uint32(0)
for i, covenantSig := range covenantSigs {
if covenantSig != nil {
covSigs[i] = covenantSig.Decrypt(decKey)
numSigs++
} else {
covSigs[i] = nil
}
if numSigs == covenantQuorum {
break
}
}
// ensure the number of covenant signatures is at least the quorum number
if numSigs < covenantQuorum {
return nil, fmt.Errorf("not enough covenant signatures to reach quorum")
}

/*
Expand Down
7 changes: 5 additions & 2 deletions x/btcstaking/types/btc_slashing_tx_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,10 @@ func FuzzSlashingTxWithWitness(f *testing.F) {
delSig, err := slashingTx.Sign(stakingMsgTx, 0, slashingPkScriptPath, delSK)
require.NoError(t, err)

covenantSigners := covenantSKs[:covenantQuorum]
// ensure that event if all covenant members provide covenant signatures,
// BuildSlashingTxWithWitness will only take a quorum number of signatures
// to construct the witness
covenantSigners := covenantSKs
// get covenant Schnorr signatures
covenantSigs, err := datagen.GenCovenantAdaptorSigs(
covenantSigners,
Expand Down Expand Up @@ -235,7 +238,7 @@ func FuzzSlashingTxWithWitness(f *testing.F) {
}

// create slashing tx with witness
slashingMsgTxWithWitness, err := slashingTx.BuildSlashingTxWithWitness(fpSK, fpBTCPKs, stakingMsgTx, 0, delSig, covSigsForFP, slashingSpendInfo)
slashingMsgTxWithWitness, err := slashingTx.BuildSlashingTxWithWitness(fpSK, fpBTCPKs, stakingMsgTx, 0, delSig, covSigsForFP, covenantQuorum, slashingSpendInfo)
require.NoError(t, err)

// verify slashing tx with witness
Expand Down
2 changes: 1 addition & 1 deletion x/btcstaking/types/btc_undelegation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func FuzzBTCUndelegation_SlashingTx(f *testing.F) {
CovenantPks: bbn.NewBIP340PKsFromBTCPKs(covenantPKs),
CovenantQuorum: covenantQuorum,
}
covenantSigners := covenantSKs[:covenantQuorum]
covenantSigners := covenantSKs

stakingTimeBlocks := uint16(5)
stakingValue := int64(2 * 10e8)
Expand Down
Loading