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

fix: address LUT issues on tests #811

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/svm/transactionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ export async function sendTransactionWithLookupTable(

// Submit the ALT creation transaction
await web3.sendAndConfirmTransaction(connection, new web3.Transaction().add(lookupTableInstruction), [sender], {
skipPreflight: true, // Avoids recent slot mismatch in simulation.
commitment: "confirmed",
skipPreflight: true,
});

// Extend the ALT with all accounts making sure not to exceed the maximum number of accounts per transaction.
Expand All @@ -51,13 +52,14 @@ export async function sendTransactionWithLookupTable(
addresses: lookupAddresses.slice(i, i + maxExtendedAccounts),
});

await web3.sendAndConfirmTransaction(connection, new web3.Transaction().add(extendInstruction), [sender], {
skipPreflight: true, // Avoids recent slot mismatch in simulation.
});
await web3.sendAndConfirmTransaction(connection, new web3.Transaction().add(extendInstruction), [sender]);
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we also set this to "confirmed", otherwise might default to "max"

Copy link
Member Author

Choose a reason for hiding this comment

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

sure & done.

}

// Avoids invalid ALT index as ALT might not be active yet on the following tx.
await new Promise((resolve) => setTimeout(resolve, 1000));
// Wait for slot to advance. LUTs only active after slot advance.
const initialSlot = await connection.getSlot();
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice finding!
Suggestion: Could we use something like this connection.onSlotChange to subscribe to slot changes? Not sure how reliable onSlotChange is though

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried this and this was better. not sure if onSlotChange was delayed or returned early but it did to result in the tests always working as expected, like this implementation does.

while ((await connection.getSlot()) === initialSlot) {
await new Promise((resolve) => setTimeout(resolve, 50));
}

// Fetch the AddressLookupTableAccount
const lookupTableAccount = (await connection.getAddressLookupTable(lookupTableAddress)).value;
Expand All @@ -76,5 +78,12 @@ export async function sendTransactionWithLookupTable(
versionedTx.sign([sender]);
const txSignature = await connection.sendTransaction(versionedTx);

// Confirm the versioned transaction
let block = await connection.getLatestBlockhash();
await connection.confirmTransaction(
{ signature: txSignature, blockhash: block.blockhash, lastValidBlockHeight: block.lastValidBlockHeight },
"confirmed"
);

return { txSignature, lookupTableAddress };
}
Loading