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: script to fill jupiter swap #819

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Reinis-FRP
Copy link
Contributor

@Reinis-FRP Reinis-FRP commented Dec 23, 2024

Adds a script to test Across+ fills with Jupiter swap integration on mainnet. Note that all remaining tokens above minimum output amount can be stolen by anyone. This could be improved by either creating a sweeper program that reads actual handler ATA balance and transfers all of them to the recipient ATA or building this functionality in the Multicall handler.

Tested on below mainnet deployments:

multicall_handler = "71cJqNV4vkmxsbk422c2KpST3aSwuvKsQ4kpLD8RBHZ5"
svm_spoke = "D1WcTLKRyN4TuqnfMBGPvc16nACHfNhFPbgWNExb1aur"

Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
Signed-off-by: Reinis Martinsons <[email protected]>
@@ -76,5 +116,33 @@ export async function sendTransactionWithLookupTable(
versionedTx.sign([sender]);
const txSignature = await connection.sendTransaction(versionedTx);

return { txSignature, lookupTableAddress };
// Confirm the versioned transaction
await confirmTransaction(connection, txSignature);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might conflict once #811 is merged in, but seems better to have reusable transaction confirmation helper

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has some overlaps from #811 as we needed better tx confirmation for filling across+

Signed-off-by: Reinis Martinsons <[email protected]>
Comment on lines +49 to +52
const svmSpokeIdl = require("../../target/idl/svm_spoke.json");
const svmSpokeProgram = new Program<SvmSpoke>(svmSpokeIdl, provider);
const handlerIdl = require("../../target/idl/multicall_handler.json");
const handlerProgram = new Program<MulticallHandler>(handlerIdl, provider);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

might want to simplify this once #806 is in

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not related to this PR, but linter picked these up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We might also consider passing priority fees to loadExecuteRelayerRefundLeafParams, but that would require some refactor not directly related to this PR

/*
* Prepends optional ComputeBudget instructions to the transaction instructions.
*/
export function prependComputeBudget(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice helpers!

data: Buffer.from(instruction.data, "base64"),
});
const innerCpiLimit = 1280;
const innerCpiSize = transactionInstruction.keys.length * 34 + transactionInstruction.data.length;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: const accountMetaSize = 34

const outputMintInfo = await provider.connection.getAccountInfo(outputMint);
if (!outputMintInfo) throw new Error("Output mint account not found");
const outputTokenProgram = new PublicKey(outputMintInfo.owner);
const recipientOutputTA = getAssociatedTokenAddressSync(outputMint, recipient, true, outputTokenProgram);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: shoud this be called recipientOutputATA?

if (!outputMintInfo) throw new Error("Output mint account not found");
const outputTokenProgram = new PublicKey(outputMintInfo.owner);
const recipientOutputTA = getAssociatedTokenAddressSync(outputMint, recipient, true, outputTokenProgram);
const handlerOutputTA = getAssociatedTokenAddressSync(outputMint, handlerSigner, true, outputTokenProgram);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: shoud this be called handlerOutputATA?

const message = new AcrossPlusMessageCoder({
handler: handlerProgram.programId,
readOnlyLen: multicallHandlerCoder.readOnlyLen,
valueAmount: new BN(valueAmount), // Must exactly cover ATA creation.
Copy link
Contributor

@md0x md0x Jan 13, 2025

Choose a reason for hiding this comment

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

Question: what happens if handlerATA or recipientATA already exist? Would this error if we send exempt rent x2 and we only create 1 or 0?
I have this question because it seems to me that createAssociatedTokenAccountIdempotentInstruction creates the ATA only if needed, otherwise it does nothing, that's why I consider the scenario of my question possible. is this correct?

relayer,
addressLookupTableAccounts
);
console.log("Fill transaction signature:", txSignature);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we log the new balances after the execution?

const slippageBps = resolvedArgv.slippageBps || 100; // default to 1%
const maxAccounts = resolvedArgv.maxAccounts || 24;
const priorityFeePrice = resolvedArgv.priorityFeePrice;
const fillComputeUnit = resolvedArgv.fillComputeUnit || 400_000;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to calculate this on the fly instead of hardcoding it or is it difficult to estimate the Jupiter transactions consumption?

Copy link
Contributor

@md0x md0x left a comment

Choose a reason for hiding this comment

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

Very cool. Added a few comments and questions for my understanding but approving already.

@chrismaree
Copy link
Member

given this POC is done, shall we merge it in so we have it within the repo?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants