From 20f169c0b0e1ca038e754860fb6cd2c1de5e839d Mon Sep 17 00:00:00 2001
From: sudhir-b <sud.balaji@gmail.com>
Date: Mon, 30 May 2022 16:47:56 +0100
Subject: [PATCH] fix: sort accountMetas after deduping

---
 web3.js/src/transaction.ts       | 24 +++++++++++++-----------
 web3.js/test/transaction.test.ts | 27 +++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/web3.js/src/transaction.ts b/web3.js/src/transaction.ts
index c868aa33074439..7d6e0eb4cd1b03 100644
--- a/web3.js/src/transaction.ts
+++ b/web3.js/src/transaction.ts
@@ -376,17 +376,6 @@ export class Transaction {
       });
     });
 
-    // Sort. Prioritizing first by signer, then by writable
-    accountMetas.sort(function (x, y) {
-      const pubkeySorting = x.pubkey
-        .toBase58()
-        .localeCompare(y.pubkey.toBase58());
-      const checkSigner = x.isSigner === y.isSigner ? 0 : x.isSigner ? -1 : 1;
-      const checkWritable =
-        x.isWritable === y.isWritable ? pubkeySorting : x.isWritable ? -1 : 1;
-      return checkSigner || checkWritable;
-    });
-
     // Cull duplicate account metas
     const uniqueMetas: AccountMeta[] = [];
     accountMetas.forEach(accountMeta => {
@@ -397,11 +386,24 @@ export class Transaction {
       if (uniqueIndex > -1) {
         uniqueMetas[uniqueIndex].isWritable =
           uniqueMetas[uniqueIndex].isWritable || accountMeta.isWritable;
+        uniqueMetas[uniqueIndex].isSigner =
+          uniqueMetas[uniqueIndex].isSigner || accountMeta.isSigner;
       } else {
         uniqueMetas.push(accountMeta);
       }
     });
 
+    // Sort. Prioritizing first by signer, then by writable
+    uniqueMetas.sort(function (x, y) {
+      const pubkeySorting = x.pubkey
+        .toBase58()
+        .localeCompare(y.pubkey.toBase58());
+      const checkSigner = x.isSigner === y.isSigner ? 0 : x.isSigner ? -1 : 1;
+      const checkWritable =
+        x.isWritable === y.isWritable ? pubkeySorting : x.isWritable ? -1 : 1;
+      return checkSigner || checkWritable;
+    });
+
     // Move fee payer to the front
     const feePayerIndex = uniqueMetas.findIndex(x => {
       return x.pubkey.equals(feePayer);
diff --git a/web3.js/test/transaction.test.ts b/web3.js/test/transaction.test.ts
index 37dd0d7df9832b..3b3eb41b754697 100644
--- a/web3.js/test/transaction.test.ts
+++ b/web3.js/test/transaction.test.ts
@@ -47,6 +47,33 @@ describe('Transaction', () => {
       expect(message.accountKeys[2]).to.eql(account3.publicKey);
     });
 
+    it('accountKeys are sorted & deduplicated correctly', () => {
+      const payer = Keypair.generate();
+      const account2 = new PublicKey(2);
+      const account3 = new PublicKey(1);
+      const recentBlockhash = Keypair.generate().publicKey.toBase58();
+      const programId = Keypair.generate().publicKey;
+      const transaction = new Transaction({
+        blockhash: recentBlockhash,
+        lastValidBlockHeight: 9999,
+      }).add({
+        keys: [
+          {pubkey: account3, isSigner: true, isWritable: false},
+          {pubkey: payer.publicKey, isSigner: true, isWritable: true},
+          {pubkey: account2, isSigner: false, isWritable: true},
+          {pubkey: account2, isSigner: true, isWritable: false},
+        ],
+        programId,
+      });
+
+      transaction.feePayer = payer.publicKey;
+
+      const message = transaction.compileMessage();
+      expect(message.accountKeys[0]).to.eql(payer.publicKey);
+      expect(message.accountKeys[1].toBase58()).to.eql(account2.toBase58());
+      expect(message.accountKeys[2].toBase58()).to.eql(account3.toBase58());
+    });
+
     it('payer is first account meta', () => {
       const payer = Keypair.generate();
       const other = Keypair.generate();