-
Notifications
You must be signed in to change notification settings - Fork 250
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
Inconsistent Signature Order in PartialSign Function #224
Comments
Test Case to Reproduce IssueHere's a simple test code that demonstrates the issue with the func TestPartialSignTransaction(t *testing.T) {
signers := []PrivateKey{
NewWallet().PrivateKey,
NewWallet().PrivateKey,
}
instructions := []Instruction{
&testTransactionInstructions{
accounts: []*AccountMeta{
{PublicKey: signers[0].PublicKey(), IsSigner: true, IsWritable: false},
{PublicKey: signers[1].PublicKey(), IsSigner: true, IsWritable: true},
},
data: []byte{0xaa, 0xbb},
programID: MustPublicKeyFromBase58("11111111111111111111111111111111"),
},
}
blockhash, err := HashFromBase58("A9QnpgfhCkmiBSjgBuWk76Wo3HxzxvDopUq9x6UUMmjn")
require.NoError(t, err)
trx, err := NewTransaction(instructions, blockhash)
require.NoError(t, err)
assert.Equal(t, trx.Message.Header.NumRequiredSignatures, uint8(2))
signatures, err := trx.PartialSign(func(key PublicKey) *PrivateKey {
if key.Equals(signers[1].PublicKey()) {
return &signers[1]
}
return nil
})
require.NoError(t, err)
assert.Equal(t, len(signatures), 1)
// full sign
signatures, err = trx.PartialSign(func(key PublicKey) *PrivateKey {
if key.Equals(signers[0].PublicKey()) {
return &signers[0]
}
return nil
})
require.NoError(t, err)
assert.Equal(t, len(signatures), 2)
require.NoError(t, trx.VerifySignatures())
} Log result
Test Code Based on Lines 110 to 134 in a5e17d6
|
Temporary Workaround for this Issuefunc PartialSignTransaction(tx *solana.Transaction, signer *solana.PrivateKey) (*solana.Signature, error) {
// Ensure that the transaction has the correct number of signatures initialized
numRequiredSignatures := tx.Message.Header.NumRequiredSignatures
if len(tx.Signatures) == 0 {
tx.Signatures = make([]solana.Signature, numRequiredSignatures)
} else if len(tx.Signatures) != int(numRequiredSignatures) {
return nil, fmt.Errorf("invalid signatures length, expected %d, actual %d", numRequiredSignatures, len(tx.Signatures))
}
fullIndex := -1 // This will track the index of the signer in the message's accounts
signerIndex := 0 // This is the expected index position of the signer's signature in the transaction
// Perform the partial signing only with the provided private key
_, err := tx.PartialSign(func(key solana.PublicKey) *solana.PrivateKey {
fullIndex++
if signer.PublicKey().Equals(key) {
signerIndex = fullIndex
return signer
}
return nil
})
if err != nil {
return nil, err
}
// Move the last signature (the one just added) to the correct position if needed
if signerIndex != len(tx.Signatures)-1 {
tx.Signatures[signerIndex] = tx.Signatures[len(tx.Signatures)-1]
tx.Signatures = tx.Signatures[:len(tx.Signatures)-1]
}
return &tx.Signatures[signerIndex], nil
} This code snippet ensures each signature is indexed correctly according to the signer's position, despite the order in which signatures are requested or provided. |
I have submitted a PR. I welcome all maintainers and contributors to review the changes. |
Description
In the current implementation of the
PartialSign
function intransaction.go
, the order of signatures does not consistently correspond with the order of signer keys. This could potentially lead to issues where signatures are incorrectly mapped to their respective public keys, especially in transactions involving multiple signers.Steps to Reproduce
Transaction
with multiple signers.PartialSign
for each signer in an order that does not match the order ofsignerKeys
.signerKeys
.Expected Behavior
The signatures in the
Transaction.Signatures
should correspond exactly in order to thesignerKeys
array regardless of the order in whichPartialSign
is called.Actual Behavior
The signatures are appended in the order that
PartialSign
is called, which may not match the order ofsignerKeys
.Possible Solution
Modify the
PartialSign
method to ensure that signatures are placed in the correct positions within theTransaction.Signatures
array according to their correspondingsignerKeys
.Additional Information
I have implemented a temporary workaround that involves manually rearranging the signatures array after each call to
PartialSign
. However, a more robust solution within thePartialSign
function would be preferable to prevent potential errors and improve code reliability.Environment
I am happy to provide further details or assist in testing any potential fixes.
The text was updated successfully, but these errors were encountered: