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

txscript: return multisig signing error for no-op #3167

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 17 additions & 8 deletions txscript/sign/sign.go
Expand Up @@ -118,15 +118,15 @@ func p2pkSignatureScript(tx *wire.MsgTx, idx int, subScript []byte,
}

// signMultiSig signs as many of the outputs in the provided multisig script as
// possible. It returns the generated script and a boolean if the script
// fulfills the contract (i.e. nrequired signatures are provided). Since it is
// arguably legal to not be able to sign any of the outputs, no error is
// returned.
// possible. It returns the generated script if 1 or more signature was provided,
// otherwise it returns detailed error (while some parts of that error message
// are expected artifacts of execution others might be useful when debugging
// signing issues).
Comment on lines -121 to +124
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it is
// arguably legal to not be able to sign any of the outputs, no error is
// returned.

Pointing out this comment, I might be missing something, but I don't see why somebody would try to sign multisig transaction unless he expects at least 1 signature to succeed.

func signMultiSig(tx *wire.MsgTx, idx int, subScript []byte,
hashType txscript.SigHashType, addresses []stdaddr.Address,
nRequired uint16, kdb KeyDB) ([]byte, bool) {
nRequired uint16, kdb KeyDB) ([]byte, error) {

// No need to add dummy in Decred.
// No need to add dummy in Decred (like for Bitcoin where it was bugged).
builder := txscript.NewScriptBuilder()
var signed uint16
for _, addr := range addresses {
Expand All @@ -147,8 +147,14 @@ func signMultiSig(tx *wire.MsgTx, idx int, subScript []byte,
}
}

// TODO - implement gathering all the errors for inspection
errs := fmt.Errorf("")
if signed == 0 {
return nil, fmt.Errorf("couldn't sign even once: %v", errs)
}

script, _ := builder.Script()
return script, signed == nRequired
return script, nil
}

// stakeSubScriptType potentially transforms the provided script type by
Expand Down Expand Up @@ -291,8 +297,11 @@ func sign(chainParams stdaddr.AddressParams, tx *wire.MsgTx, idx int,
case stdscript.STMultiSig:
details := stdscript.ExtractMultiSigScriptDetailsV0(subScript, false)
threshold := details.RequiredSigs
script, _ := signMultiSig(tx, idx, subScript, hashType, addresses,
script, err := signMultiSig(tx, idx, subScript, hashType, addresses,
threshold, kdb)
if err != nil {
return nil, scriptType, nil, err
}
return script, scriptType, addresses, nil

case stdscript.STStakeSubmissionPubKeyHash,
Expand Down