Skip to content

Conversation

@Raunaque97
Copy link
Contributor

@Raunaque97 Raunaque97 commented Nov 25, 2025

closes #340

  • refactored minaTransactionSender to use a persistent Storage.
  • added a simple retry Strategy to increase fees by a multiple and retry every few minutes if the transaction is not included.
  • also handles the case if the sequencer restarts and the last sent transaction statuses were not updated in DB.

@rpanic
Copy link
Member

rpanic commented Nov 26, 2025

So what I just discovered: When we setFee(), o1js erases all previous signatures in order for the tx to be signed again. And this makes sense - for the feepayer and some accountupdates, o1js uses the so-called "full commitment". This is simply just the hash of the whole transaction, including all accountupdates and the feepayer, whereas the non-full commitment would just the be hash of the current accountupdate.
Now, accountupdates can actually specify whether they want to use the full commitment or not (this is a field in the accountupdate struct that mina will receive as part of the transaction). And whatever commitment you choose, you signature or proof will only be valid given that exact commitment. it is kinda the basis on which you sign or prove. Now why is this important?
Signing or proving over a full commitment prevent replay attacks, like in other chains. But mina is a bit special, and unlike other chains, we sign a bunch of sub-parts individually. And so generally, in order to prevent somebody to rip out a part of our tx from the L1 mempool and replay it in their own specially-crafted malicious transaction and frontrun ours, we sign over the full commitment.
For resending transactions with a higher fee, this is a bit annoying - we don't want to re-prove the entire tx just because we changed the fee, right?
As I've mentioned above, luckily we can choose to opt-out of using the full commitment and use only the current accountupdate's commitment instead. But this is only possible in some distinct scenarios. My initial suspicious is that this is the case iff the accountupdate's validity is directly dependent on exclusively other full-commitment based accountupdates within the same transaction. But I'll have to think more on that definition.
Anyways, what we have to do before this PR will work without re-proving some accountupdates, is to use no full commitment generally. And for us to be able to do this, we need to examine the protocol and see if we can do this safely

Copy link
Member

@rpanic rpanic left a comment

Choose a reason for hiding this comment

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

This is great! I have a few minor requests and questions, but I like it generally

messages IncomingMessageBatchTransaction[]
}

model PendingL1Transaction {
Copy link
Member

Choose a reason for hiding this comment

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

You forgot to create a migration for this new schema change - do this by cd-ing into the persistence pkg, then run prisma migrate dev and it'll you for a name and migrate

const currentFee = tx.transaction.feePayer.body.fee;
const newFee = UInt64.from(this.bumpFee(Number(currentFee.toBigInt())));
tx.setFee(newFee);
// Delay if needed
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain what this delay here accomplishes? I'd expect this function to only craft the new transaction and return it...

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 is used to add a delay between retries, like retrying a transaction every 5 minutes.

lastError String?
sentAt DateTime?
@@id([sender, nonce])
Copy link
Member

Choose a reason for hiding this comment

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

I'm not against this schema, but I think for the reorg support we'll have to change this - but that will happen when we tackle that piece of work.

lastError String?
sentAt DateTime?
@@id([sender, nonce])
Copy link
Member

Choose a reason for hiding this comment

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

What I'd love to see would be if we could add foreign-key out of settlement here. So in the Settlement entity, add a reference to the PendingTransaction so that we can establish that link

const tx = Transaction.fromJSON(JSON.parse(record.transactionJson));
const currentFee = tx.transaction.feePayer.body.fee;
const newFee = UInt64.from(this.bumpFee(Number(currentFee.toBigInt())));
tx.setFee(newFee);
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned in the comment earlier, we need to figure something out to prevent re-proving

@Raunaque97 Raunaque97 force-pushed the raun/retryMinaTransactions branch from 87ebda9 to 2fe0464 Compare December 18, 2025 13:32
@Raunaque97 Raunaque97 marked this pull request as ready for review December 18, 2025 13:51
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.

Handling of non-inclusion of L1 transactions due to low fee

3 participants