Begin to add missing unit tests around the very complicated static ac…

…count / accounts from lookup logic also found a bug with a disabled test
LMAX-Exchange · Aug 30, 2024 · 2500a9f · 2500a9f
1 parent 9ef0cde
commit 2500a9f
Show file tree

Hide file tree

Showing 5 changed files with 241 additions and 9 deletions.
diff --git a/src/main/java/com/lmax/solana4j/encoding/SolanaAccountsView.java b/src/main/java/com/lmax/solana4j/encoding/SolanaAccountsView.java
@@ -35,8 +35,7 @@ public List<PublicKey> allAccounts(final AddressesFromLookup addressesFromLookup
             throw new IllegalArgumentException("Cannot evaluate accounts, missing account lookup values");
         }
 
-        final List<PublicKey> accounts = new ArrayList<>();
-        accounts.addAll(staticAccounts);
+        final List<PublicKey> accounts = new ArrayList<>(staticAccounts);
         addressesFromLookup.write(accounts);
 
         return accounts;

diff --git a/src/main/java/com/lmax/solana4j/encoding/SolanaSealedMessageBuilder.java b/src/main/java/com/lmax/solana4j/encoding/SolanaSealedMessageBuilder.java
@@ -18,7 +18,6 @@ final class SolanaSealedMessageBuilder implements SealedMessageBuilder
     @Override
     public SignedMessageBuilder signed()
     {
-
         return new SolanaSignedMessageBuilder(buffer);
     }
 

diff --git a/src/test/java/com/lmax/solana4j/Solana4jTestHelper.java b/src/test/java/com/lmax/solana4j/Solana4jTestHelper.java
@@ -46,6 +46,9 @@ public class Solana4jTestHelper
     public static final byte[] SIGNATURE6 = newBlob(SIGNATURE_LENGTH, (byte) 22);
     public static final byte[] SIGNATURE7 = newBlob(SIGNATURE_LENGTH, (byte) 23);
     public static final byte[] SIGNATURE8 = newBlob(SIGNATURE_LENGTH, (byte) 24);
+    public static final byte[] ACCOUNT_LOOKUP_TABLE1 = newBlob(ACCOUNT_LENGTH, (byte) 25);
+    public static final byte[] ACCOUNT_LOOKUP_TABLE2 = newBlob(ACCOUNT_LENGTH, (byte) 26);
+    public static final byte[] ACCOUNT_LOOKUP_TABLE3 = newBlob(ACCOUNT_LENGTH, (byte) 27);
     public static final byte[] UNSIGNED = newBlob(SIGNATURE_LENGTH, (byte) -77);
 
     public static final Map<PublicKey, byte[]> SIGNINGS = Map.of(

diff --git a/src/test/java/com/lmax/solana4j/encoding/SolanaAccountsTest.java b/src/test/java/com/lmax/solana4j/encoding/SolanaAccountsTest.java
@@ -0,0 +1,229 @@
+package com.lmax.solana4j.encoding;
+
+import com.lmax.solana4j.api.Accounts;
+import com.lmax.solana4j.api.AddressLookupTable;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static com.lmax.solana4j.Solana4jTestHelper.ACCOUNT1;
+import static com.lmax.solana4j.Solana4jTestHelper.ACCOUNT2;
+import static com.lmax.solana4j.Solana4jTestHelper.ACCOUNT3;
+import static com.lmax.solana4j.Solana4jTestHelper.ACCOUNT4;
+import static com.lmax.solana4j.Solana4jTestHelper.ACCOUNT_LOOKUP_TABLE1;
+import static com.lmax.solana4j.Solana4jTestHelper.ACCOUNT_LOOKUP_TABLE2;
+import static com.lmax.solana4j.Solana4jTestHelper.ACCOUNT_LOOKUP_TABLE3;
+import static com.lmax.solana4j.Solana4jTestHelper.DATA1;
+import static com.lmax.solana4j.Solana4jTestHelper.DATA2;
+import static com.lmax.solana4j.Solana4jTestHelper.PAYER;
+import static com.lmax.solana4j.Solana4jTestHelper.PROGRAM1;
+import static com.lmax.solana4j.Solana4jTestHelper.PROGRAM2;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+
+class SolanaAccountsTest
+{
+    @Test
+    void withNoLookupAccountsAllAccountsAreStatic()
+    {
+        final SolanaTransactionInstruction solanaTransactionInstruction = new SolanaTransactionInstruction(
+                List.of(new SolanaAccountReference(new SolanaAccount(ACCOUNT1), true, true, false)),
+                new SolanaAccount(PROGRAM1),
+                10,
+                w -> w.put(DATA1));
+
+        final Accounts accounts = SolanaAccounts.create(
+                List.of(solanaTransactionInstruction),
+                // no account table lookups
+                List.of(),
+                new SolanaAccountReference(new SolanaAccount(PAYER), true, true, false)
+        );
+
+        assertThat(accounts.getStaticAccounts().size()).isEqualTo(3);
+        assertThat(accounts.getStaticAccounts())
+                .usingRecursiveComparison()
+                .ignoringCollectionOrder()
+                .isEqualTo(List.of(new SolanaAccount(ACCOUNT1), new SolanaAccount(PROGRAM1), new SolanaAccount(PAYER)));
+        assertThat(accounts.getLookupAccounts().size()).isEqualTo(0);
+    }
+
+    @Test
+    void ifAccountSignerAlwaysStaticAccountEvenIfInLookupTable()
+    {
+        final AddressLookupTable addressLookupTable = new SolanaAddressLookupTable(
+                new SolanaAccount(ACCOUNT_LOOKUP_TABLE1),
+                List.of(new SolanaAccount(ACCOUNT1))
+        );
+
+        final SolanaTransactionInstruction solanaTransactionInstruction = new SolanaTransactionInstruction(
+                List.of(new SolanaAccountReference(new SolanaAccount(ACCOUNT1), true, true, false)),
+                new SolanaAccount(PROGRAM1),
+                10,
+                w -> w.put(DATA1));
+
+
+        final Accounts accounts = SolanaAccounts.create(
+                List.of(solanaTransactionInstruction),
+                List.of(addressLookupTable),
+                new SolanaAccountReference(new SolanaAccount(PAYER), true, true, false)
+        );
+
+        assertThat(accounts.getStaticAccounts().size()).isEqualTo(3);
+        assertThat(accounts.getStaticAccounts())
+                .usingRecursiveComparison()
+                .ignoringCollectionOrder()
+                .isEqualTo(List.of(new SolanaAccount(ACCOUNT1), new SolanaAccount(PROGRAM1), new SolanaAccount(PAYER)));
+        assertThat(accounts.getLookupAccounts().size()).isEqualTo(0);
+    }
+
+    @Test
+    void ifAccountNotSignerAndInLookupTableThenNoLongerStaticAccountForReadWriteAccount()
+    {
+        final AddressLookupTable addressLookupTable1 = new SolanaAddressLookupTable(new SolanaAccount(ACCOUNT_LOOKUP_TABLE1), List.of(new SolanaAccount(ACCOUNT1)));
+
+        final SolanaTransactionInstruction solanaTransactionInstruction = new SolanaTransactionInstruction(
+                List.of(
+                        new SolanaAccountReference(new SolanaAccount(ACCOUNT1), false, true, false)
+                ),
+                new SolanaAccount(PROGRAM1),
+                10,
+                w -> w.put(DATA1));
+
+
+        final Accounts accounts = SolanaAccounts.create(
+                List.of(solanaTransactionInstruction),
+                List.of(addressLookupTable1),
+                new SolanaAccountReference(new SolanaAccount(PAYER), true, true, false)
+        );
+
+        assertThat(accounts.getStaticAccounts().size()).isEqualTo(2);
+        assertThat(accounts.getStaticAccounts()).usingRecursiveComparison().ignoringCollectionOrder().isEqualTo(List.of(new SolanaAccount(PROGRAM1), new SolanaAccount(PAYER)));
+        assertThat(accounts.getLookupAccounts().size()).isEqualTo(1);
+        assertThat(accounts.getLookupAccounts().get(0).getLookupTableAddress())
+                .usingRecursiveComparison()
+                .ignoringCollectionOrder()
+                .isEqualTo(new SolanaAccount(ACCOUNT_LOOKUP_TABLE1));
+        assertThat(accounts.getLookupAccounts().get(0).getAddresses().size()).isEqualTo(1);
+        assertThat(accounts.getLookupAccounts().get(0).getAddresses().get(0)).isEqualTo(new SolanaAccount(ACCOUNT1));
+        // ACCOUNT1 isWriter: true
+        assertThat(accounts.getLookupAccounts().get(0).getReadOnlyAddressIndexes().size()).isEqualTo(0);
+        assertThat(accounts.getLookupAccounts().get(0).getReadWriteAddressIndexes().size()).isEqualTo(1);
+        assertThat(accounts.getLookupAccounts().get(0).getReadWriteAddressIndexes().get(0).getAddress()).isEqualTo(new SolanaAccount(ACCOUNT1));
+        assertThat(accounts.getLookupAccounts().get(0).getReadWriteAddressIndexes().get(0).getAddressIndex()).isEqualTo(0);
+    }
+
+    @Test
+    void ifAccountNotSignerAndInLookupTableThenNoLongerStaticAccountForReadOnlyAccount()
+    {
+        final AddressLookupTable addressLookupTable1 = new SolanaAddressLookupTable(new SolanaAccount(ACCOUNT_LOOKUP_TABLE1), List.of(new SolanaAccount(ACCOUNT1)));
+
+        final SolanaTransactionInstruction solanaTransactionInstruction = new SolanaTransactionInstruction(
+                List.of(
+                        new SolanaAccountReference(new SolanaAccount(ACCOUNT1), false, false, false)
+                ),
+                new SolanaAccount(PROGRAM1),
+                10,
+                w -> w.put(DATA1));
+
+
+        final Accounts accounts = SolanaAccounts.create(
+                List.of(solanaTransactionInstruction),
+                List.of(addressLookupTable1),
+                new SolanaAccountReference(new SolanaAccount(PAYER), true, true, false)
+        );
+
+        assertThat(accounts.getStaticAccounts().size()).isEqualTo(2);
+        assertThat(accounts.getStaticAccounts())
+                .usingRecursiveComparison()
+                .ignoringCollectionOrder()
+                .isEqualTo(List.of(new SolanaAccount(PROGRAM1), new SolanaAccount(PAYER)));
+        assertThat(accounts.getLookupAccounts().size()).isEqualTo(1);
+        assertThat(accounts.getLookupAccounts().get(0).getLookupTableAddress())
+                .usingRecursiveComparison()
+                .ignoringCollectionOrder()
+                .isEqualTo(new SolanaAccount(ACCOUNT_LOOKUP_TABLE1));
+        assertThat(accounts.getLookupAccounts().get(0).getAddresses().size()).isEqualTo(1);
+        assertThat(accounts.getLookupAccounts().get(0).getAddresses().get(0)).isEqualTo(new SolanaAccount(ACCOUNT1));
+        // ACCOUNT1 isWriter: false
+        assertThat(accounts.getLookupAccounts().get(0).getReadOnlyAddressIndexes().size()).isEqualTo(1);
+        assertThat(accounts.getLookupAccounts().get(0).getReadOnlyAddressIndexes().get(0).getAddress()).isEqualTo(new SolanaAccount(ACCOUNT1));
+        assertThat(accounts.getLookupAccounts().get(0).getReadOnlyAddressIndexes().get(0).getAddressIndex()).isEqualTo(0);
+        assertThat(accounts.getLookupAccounts().get(0).getReadWriteAddressIndexes().size()).isEqualTo(0);
+    }
+
+    @Test
+    void accountsAreDeduplicatedIfTheyAppearInManyTransactions()
+    {
+        final SolanaTransactionInstruction solanaTransactionInstruction1 = new SolanaTransactionInstruction(
+                List.of(new SolanaAccountReference(new SolanaAccount(ACCOUNT1), true, true, false)),
+                new SolanaAccount(PROGRAM1),
+                10,
+                w -> w.put(DATA1));
+
+        final SolanaTransactionInstruction solanaTransactionInstruction2 = new SolanaTransactionInstruction(
+                List.of(new SolanaAccountReference(new SolanaAccount(ACCOUNT1), true, true, false)),
+                new SolanaAccount(PROGRAM2),
+                15,
+                w -> w.put(DATA2));
+
+        final Accounts accounts = SolanaAccounts.create(
+                List.of(solanaTransactionInstruction1, solanaTransactionInstruction2),
+                // no account table lookups
+                List.of(),
+                new SolanaAccountReference(new SolanaAccount(PAYER), true, true, false)
+        );
+
+        assertThat(accounts.getStaticAccounts().size()).isEqualTo(4);
+        assertThat(accounts.getStaticAccounts())
+                .usingRecursiveComparison()
+                .ignoringCollectionOrder()
+                .isEqualTo(List.of(new SolanaAccount(ACCOUNT1), new SolanaAccount(PROGRAM2), new SolanaAccount(PROGRAM1), new SolanaAccount(PAYER)));
+        assertThat(accounts.getLookupAccounts().size()).isEqualTo(0);
+    }
+
+    @Test
+    @Disabled
+    void ifAccountExistsInManyLookupTablesSelectTheIndexOfTheFirstLookupTableItsFoundIn()
+    {
+        final AddressLookupTable addressLookupTable1 = new SolanaAddressLookupTable(
+                new SolanaAccount(ACCOUNT_LOOKUP_TABLE1),
+                List.of(new SolanaAccount(ACCOUNT2), new SolanaAccount(ACCOUNT3))
+        );
+        final AddressLookupTable addressLookupTable2 = new SolanaAddressLookupTable(
+                new SolanaAccount(ACCOUNT_LOOKUP_TABLE2),
+                List.of(new SolanaAccount(ACCOUNT4), new SolanaAccount(ACCOUNT1))
+        );
+        final AddressLookupTable addressLookupTable3 = new SolanaAddressLookupTable(
+                new SolanaAccount(ACCOUNT_LOOKUP_TABLE3),
+                List.of(new SolanaAccount(ACCOUNT1))
+        );
+
+        final SolanaTransactionInstruction solanaTransactionInstruction1 = new SolanaTransactionInstruction(
+                List.of(new SolanaAccountReference(new SolanaAccount(ACCOUNT1), false, false, false)),
+                new SolanaAccount(PROGRAM1),
+                10,
+                w -> w.put(DATA1));
+
+        final Accounts accounts = SolanaAccounts.create(
+                List.of(solanaTransactionInstruction1),
+                List.of(addressLookupTable1, addressLookupTable2, addressLookupTable3),
+                new SolanaAccountReference(new SolanaAccount(PAYER), true, true, false)
+        );
+
+        assertThat(accounts.getStaticAccounts().size()).isEqualTo(2);
+        assertThat(accounts.getStaticAccounts()).usingRecursiveComparison().ignoringCollectionOrder().isEqualTo(List.of(new SolanaAccount(PROGRAM1), new SolanaAccount(PAYER)));
+        // fails here ... it's actually 2, i'm not sure it should be
+        assertThat(accounts.getLookupAccounts().size()).isEqualTo(1);
+        assertThat(accounts.getLookupAccounts().get(0).getLookupTableAddress())
+                .usingRecursiveComparison()
+                .ignoringCollectionOrder()
+                .isEqualTo(new SolanaAccount(ACCOUNT_LOOKUP_TABLE2));
+        assertThat(accounts.getLookupAccounts().get(0).getAddresses().size()).isEqualTo(1);
+        assertThat(accounts.getLookupAccounts().get(0).getAddresses().get(0)).isEqualTo(new SolanaAccount(ACCOUNT1));
+        // ACCOUNT1 isWriter: false
+        assertThat(accounts.getLookupAccounts().get(0).getReadOnlyAddressIndexes().size()).isEqualTo(1);
+        assertThat(accounts.getLookupAccounts().get(0).getReadOnlyAddressIndexes().get(0).getAddress()).isEqualTo(new SolanaAccount(ACCOUNT1));
+        assertThat(accounts.getLookupAccounts().get(0).getReadOnlyAddressIndexes().get(0).getAddressIndex()).isEqualTo(2);
+        assertThat(accounts.getLookupAccounts().get(0).getReadWriteAddressIndexes().size()).isEqualTo(0);
+    }
+}
diff --git a/src/test/java/com/lmax/solana4j/encoding/SolanaMessageFormattingCommonTest.java b/src/test/java/com/lmax/solana4j/encoding/SolanaMessageFormattingCommonTest.java
@@ -15,6 +15,8 @@
 import static com.lmax.solana4j.Solana4jTestHelper.ACCOUNT5;
 import static com.lmax.solana4j.Solana4jTestHelper.ACCOUNT6;
 import static com.lmax.solana4j.Solana4jTestHelper.ACCOUNT7;
+import static com.lmax.solana4j.Solana4jTestHelper.ACCOUNT_LOOKUP_TABLE1;
+import static com.lmax.solana4j.Solana4jTestHelper.ACCOUNT_LOOKUP_TABLE2;
 import static com.lmax.solana4j.Solana4jTestHelper.BLOCKHASH;
 import static com.lmax.solana4j.Solana4jTestHelper.DATA1;
 import static com.lmax.solana4j.Solana4jTestHelper.DATA2;
@@ -91,16 +93,16 @@ void readsSignatures()
     @Test
     void writesLookupAccounts()
     {
-        // ACCOUNT1 is the lookup table address
-        final SolanaAddressLookupTableIndexes lookupTable1 = new SolanaAddressLookupTableIndexes(new SolanaAccount(ACCOUNT1));
+        // ACCOUNT_LOOKUP_TABLE1 is the lookup table address
+        final SolanaAddressLookupTableIndexes lookupTable1 = new SolanaAddressLookupTableIndexes(new SolanaAccount(ACCOUNT_LOOKUP_TABLE1));
         // ACCOUNT3 is the address referenced at the lookup table index 1
         lookupTable1.addReadOnlyIndex(new SolanaAccount(ACCOUNT3), 1);
         // ACCOUNT4 is the address referenced at the lookup table index 2
         lookupTable1.addReadWriteIndex(new SolanaAccount(ACCOUNT4), 2);
         lookupTable1.addReadWriteIndex(new SolanaAccount(ACCOUNT5), 3);
 
-        // ACCOUNT2 is the lookup table address
-        final SolanaAddressLookupTableIndexes lookupTable2 = new SolanaAddressLookupTableIndexes(new SolanaAccount(ACCOUNT2));
+        // ACCOUNT_LOOKUP_TABLE1 is the lookup table address
+        final SolanaAddressLookupTableIndexes lookupTable2 = new SolanaAddressLookupTableIndexes(new SolanaAccount(ACCOUNT_LOOKUP_TABLE2));
         // ACCOUNT5 is the address referenced at the lookup table index 1
         lookupTable2.addReadOnlyIndex(new SolanaAccount(ACCOUNT6), 4);
         // ACCOUNT6 is the address referenced at the lookup table index 2
@@ -115,7 +117,7 @@ void writesLookupAccounts()
 
         // lookup table 1 address
         final AccountLookupTableView accountLookupTable1 = accountLookupTableViews.get(0);
-        assertThat(accountLookupTable1.lookupAccount()).isEqualTo(new SolanaAccount(ACCOUNT1));
+        assertThat(accountLookupTable1.lookupAccount()).isEqualTo(new SolanaAccount(ACCOUNT_LOOKUP_TABLE1));
 
         assertThat(accountLookupTable1.readOnlyTableIndexes().size()).isEqualTo(1);
         assertThat(accountLookupTable1.readOnlyTableIndexes().get(0)).isEqualTo(1);
@@ -125,7 +127,7 @@ void writesLookupAccounts()
 
         // lookup table 2 address
         final AccountLookupTableView accountLookupTable2 = accountLookupTableViews.get(1);
-        assertThat(accountLookupTable2.lookupAccount()).isEqualTo(new SolanaAccount(ACCOUNT2));
+        assertThat(accountLookupTable2.lookupAccount()).isEqualTo(new SolanaAccount(ACCOUNT_LOOKUP_TABLE2));
 
         assertThat(accountLookupTable2.readOnlyTableIndexes().size()).isEqualTo(1);
         assertThat(accountLookupTable2.readOnlyTableIndexes().get(0)).isEqualTo(4);
-Original file line number
+Diff line change
@@ Expand Up @@
         @Override
         public SignedMessageBuilder signed()
         {
             return new SolanaSignedMessageBuilder(buffer);
         }
@@ Expand Down @@