[Bug] Transaction signature verification failure

[robertvo]

Describe the bug Get error from Solana network "Transaction signature verification failure"

To Reproduce Create a consolidation transaction with 4 transfers from 4 input addresses to 1 output

There's some kind of signature mismatch. It always works with 1 input, it SOMETIMES works with 3 and it never works with 4.

[AbanoubNassem]

any updates ? have you figured a work around ?

[BifrostTitan]

This is from Solnet.Serum. Maybe it will help bypass the verification issue. When certain transactions are deserialized the public keys for the signatures are pulled from the instructions which can be incorrect.

Create a TransactionBuilder instance and create your transaction by adding the instructions etc then obtain signatures from all the signers and add them to the signature list. Try populating the signatures manually.

I will look into the web3.js source code and take a look at the typescript implementation to see if I can find a better way to serialize and break down transactions.

   byte[] txBytes = txBuilder.CompileMessage();

   byte[] signatureBytes = TraderWallet.Sign(txBytes);

   List<byte[]> signatures = new() { signatureBytes };
   signatures.AddRange(_signers.Select(signer => signer.Sign(txBytes)));
   _signers.Clear();

   Transaction tx = Transaction.Populate(Message.Deserialize(txBytes), signatures);
    return await rpc.SendTransactionAsync(tx.Serialize());

[robertvo]

I did not figure this out. Instead of making 4 transfers in one transactions, I'm sending 4 separate transactions :(

[pfleeter]

I believe I have a clue for this.

I have found that when I add signers, success becomes less likely. I further demonstrated in my code that when serializing a transaction, the order of the accounts and the order of the signatures are not matched, which is required.

I wrote the following unit test, which just creates a transaction with increasing numbers of signers and does a "waterfall" transfer of 1 lamport between them. So, if there is 1 transfer, there are two signers and 3 accounts. signers: fee payer, a sender; non-signer a receiver. If there were two transfers there is fee payer A, B -> C, then C->D (so 3 signers: A, B and C) and so on.

This is repeated from 1 transfer to 7. At 1 transfer, success is about 100% (1000 iterations). By the time you get to 7 it will sometimes succeed 1 / 1000 times.

When printing the success cases, it appears that if the non-fee paying signers happen to be in alphabetical order, it works.

Here are the logs from this test code:

Transfers: 1, Success Count: 1000 Alphabetical Non-Fee Payer Signers Count: 1000 Transfers: 2, Success Count: 497 Alphabetical Non-Fee Payer Signers Count: 497 Transfers: 3, Success Count: 176 Alphabetical Non-Fee Payer Signers Count: 176 Transfers: 4, Success Count: 37 Alphabetical Non-Fee Payer Signers Count: 37 Transfers: 5, Success Count: 8 Alphabetical Non-Fee Payer Signers Count: 8 Transfers: 6, Success Count: 1 Alphabetical Non-Fee Payer Signers Count: 1 Transfers: 7, Success Count: 0 Alphabetical Non-Fee Payer Signers Count: 0

record IterationStats(int SuccessCount, int AlphabetizedCount);

[Fact]
public async Task TestSolnetTransactionSigningSerialization()
{
    var successCounts = new Dictionary<int, IterationStats>();
    var attemptCount = 1000;
    var minTransfers = 1;
    var maxTransfers = 7;

    for (var transfers = minTransfers; transfers <= maxTransfers; transfers++)
    {
        var successCount = 0;
        var alphabetizedCount = 0;

        for (var i = 0; i < attemptCount; i++)
        {
            var numberOfAccounts = transfers + 2; // fee payer, sender, receiver
            var accountList = new List<Account>(numberOfAccounts);
            for (var j = 0; j <= numberOfAccounts; j++)
            {
                accountList.Add(new Account());
            }
        
            var transaction = new Transaction
            {
                FeePayer = accountList[0].PublicKey
            };

            for (var j = 1; j <= transfers; j++)
            {
                transaction.Add(SystemProgram.Transfer(accountList[j], accountList[j+1], 1));
            }
          
            // just something
            transaction.RecentBlockHash = "BtQ4EEbRrNbFiMf3g16hdhkN9VLSwBtYjbdrW5rDar5A";

            var signersList = new List<Account>(transfers + 1);
            for (var j = 0; j <= transfers; j++)
            {
                signersList.Add(accountList[j]);
            }
            transaction.Sign(signersList);

            // In this case, things always succeed because the signers are pulled from the signers list as they are added
            // above
            Assert.True(transaction.VerifySignatures());

            // Hypothesis: serialization only creates an accounts list that matches the order of the serialized signers
            // when the non-fee payer signers are in alphabetical order
   
            var signersPublicKeys = signersList.Select(s => s.PublicKey.ToString()).ToList();
            // Get all but the first signers (ignore the fee payer)
            var nonFeePayerSignerPublicKey = signersPublicKeys.Slice(1, signersPublicKeys.Count - 1).ToList();
            var sortedSignersList = nonFeePayerSignerPublicKey.Select(s => s).OrderBy(x => x).ToList();
            var signersListWasAlphabetical = nonFeePayerSignerPublicKey.SequenceEqual(sortedSignersList);
            if (signersListWasAlphabetical)
            {
                // The non-fee payer signers happened to be in alphabetical order
                alphabetizedCount++;
            }
            
            var bytes = transaction.Serialize();

            var deserializedTransaction = Transaction.Deserialize(bytes);

            // This may fail, because the signers have to be loaded in from the serialized message, in which the 
            // signers header may be in a different order than the accounts list
            if (deserializedTransaction.VerifySignatures())
            {
                successCount++;
                
                if (transfers == maxTransfers)
                {
                    _testOutputHelper.WriteLine($"Success w/ transfer count: {transfers}.  " +
                                                $"Signer keys in order were: {string.Join("\n", signersPublicKeys)}");
                }
            }
        }
        
        successCounts.Add(transfers, new IterationStats(successCount, alphabetizedCount));
    
        // Assert.Equal(attemptCount, successCount);
    }
    
    foreach (var (transfers, iterationStats) in successCounts)
    {
        _testOutputHelper.WriteLine(
            $"Transfers: {transfers}, " +
            $"Success Count: {iterationStats.SuccessCount} " +
            $"Alphabetical Non-Fee Payer Signers Count: {iterationStats.AlphabetizedCount}");
    }
}

[linbin524]

I also conducted a multi-signature test in the DevNet network environment. I found that when n = 5 and m = 3, the number of signatures for sending a transaction can only be in combinations of three. Moreover, some signers in these three - signature combinations still fail, and I have to keep testing to identify them. This contradicts the original expectation that sending a transaction with any number of signatures greater than m and up to n should succeed. I hope this can be improved.

One of my tests:

(1) Step 1: Create 5 signers and 1 multi - signature initiator. 2exbrgkr4zbenekr5QXYtxsDhpZAp4jsKZRaiS3yPRVM3LukmJqePQPNdQQVgpK44uj91FmtBiMLoKbeJPVXReCp

{
    "info": {
        "m": 3,
        "multisig": "DmyGaJf1CNHTUAkCK59e4Y4BXoPcxLp4rxcp5TSQGgab",
        "rentSysvar": "SysvarRent111111111111111111111111111111111",
        "signers": [
            "5TKQ6izmFeBQjZqN3TkWsLZXvyhEQKCTxpfVgeyGxfGb",
            "69H194YonbJ4YVwGriLARrG96CiPcnyBEwxtTam7sNZw",
            "Cfae3iwVqLpEQGHUZX5skZwsYA5T2P6ALF7Rc7HVPN7w",
            "25DzYL6mqF6k6aav2iHbCjqZWrVvGrowa6RpvKeH7DgD",
            "C88fSFcArJMeQFpoaVUF5gbWtHTJGEwEGsSRJ9EdEAKD"
        ]
    },
    "type": "initializeMultisig"
}

2. Transfer a specified amount of tokens to the multi - signature account. Transaction return: 4AAJaHvTzuYkVaA7daD7NDY6LhEf5qZq44fj5NmVkA8a2yC7bFEW3fzEejMGtaggbyuFJ3YxX2G6kNahy7zDyaHV

3. Use the multi - signature initiator to transfer tokens to a certain user and use multiple signers to sign. The successful result is: Transaction return: 317zZVDpqNsG1W2fYSHWnAtRYT1vveDnxkKHu7ZbADukbCzQmBaSYpx8Y8kb1CDwi3KmnJqFC7SoTsKcsrqrdiUv

Among the signatures, some combinations work, while others don't, as follows: Combinations of signers 2, 3, and 4 work. Combinations of signers 2, 3, and 6 work. Combinations of signers 2, 3, and 5 don't work. Combinations of signers 4, 5, and 6 don't work.

Note: sign2 = 5TKQ6izmFeBQjZqN3TkWsLZXvyhEQKCTxpfVgeyGxfGb sign3 = 69H194YonbJ4YVwGriLARrG96CiPcnyBEwxtTam7sNZw sign4 = Cfae3iwVqLpEQGHUZX5skZwsYA5T2P6ALF7Rc7HVPN7w sign5 = 25DzYL6mqF6k6aav2iHbCjqZWrVvGrowa6RpvKeH7DgD sign6 = C88fSFcArJMeQFpoaVUF5gbWtHTJGEwEGsSRJ9EdEAKD

[BifrostTitan]

@pfleeter Which version of solnet are you using?

[pfleeter]

@BifrostTitan Sorry, I never saw your question. But I was reminded of this as I came across a comment in our code base. We are using 6.1.0

I should also say that this hack has been working for us:

        ///////////////////////////////////////////////////////////////////////////////
        // HACK https://github.com/bmresearch/Solnet/issues/459#issuecomment-2657080413
        // Solnet has issues serializing transactions if the non fee paying accounts
        // are not in alphabetical order
        ///////////////////////////////////////////////////////////////////////////////
        signers = new[] { payer }
            .Concat(signers
                .Where(a => a.PublicKey != payer.PublicKey)
                .OrderBy(a => a.PublicKey.ToString()))
            .ToList();
        ///////////////////////////////////////////////////////////////////////////////
        // END HACK
        ///////////////////////////////////////////////////////////////////////////////
        
        // Sign it and extract the transaction signature
        transaction.Sign(signers);

[BifrostTitan]

Patched #502