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

[rfc] Add idempotence tokens to API to prevent duplicate transactions #1

Open
steveluscher opened this issue Jan 1, 2022 · 5 comments

Comments

@steveluscher
Copy link

Preamble

The internet is a jerk. Sometimes connections go down. Requests rebroadcast as clients retry. Retry logic is often unsophisticated.

Problem statement

We have logic in Octane to prevent malicious consecutive signing requests (through locks) but we might also consider protecting against accidental dupes.

There are many reasons why a client might accidentally re-send a request.

  • It disconnected after sending the first request, but before receiving the response.
  • A fatal in the response handler re-triggered the request.
  • It's dumb.

Proposal

Require, as part of the transaction signing request API, that clients supply an idempotence token. Octane would store this token in a distributed storage system like Upstash (Redis). If Octane encounters a signing request having an idempotence token that it has seen before, it drops the request.

Details

  • It's important that the idempotence token be universally unique. In practice, this will probably look something like Octane taking whatever #yolo idempotence token the client sends and hashing it together with the transaction itself to create something unique. If a client insists on reusing idempotence tokens (eg. '') multiple times with the exact same transaction, it's gonna have a bad time.
  • The distributed data store probably needs to store three states for each idempotence token:
    • Nothing stored (never seen this transaction).
    • in-flight when the transaction has been received and validated, but not yet confirmed.
    • expended when the transaction associated with this idempotence token has been confirmed.
@jordaaash
Copy link
Collaborator

The final transaction signature (aka txid) itself should be sufficient for this -- the signature incorporates the recent blockhash: https://docs.solana.com/developing/programming-model/transactions#recent-blockhash

Any transaction that is completely identical to a previous one is rejected

Assuming we use a mutex (#2) to store signatures we've seen, I think we can use this to uniquely represent a transaction. I don't think we need a state for it, as it's either in flight, or it's on chain (confirmed or rejected).

I expect that simulation of a duplicate signature should fail, but this should be tested.

@jordaaash
Copy link
Collaborator

904343d from #3 has a possible implementation of this (with the same caveats as #2) but at least here it's not necessary (any such transaction will already be locked out by its source token account).

@steveluscher
Copy link
Author

Any transaction that is completely identical to a previous one is rejected

Oh, that's awesome. That gives us idempotence at the chain-level.

If the presence of a recent blockhash (or nonce info) in the transaction implies that a hash of the transaction itself is an idempotence token for the transaction, how about we use that hash of the transaction itself as a lock for Octane?

We could detect the presence of any such lock in middleware which would save Octane from doing any work at all. No validating the transaction, no validating the transfer; nothing.

To prevent that list from growing indefinitely, we could set a TTL roughly as long as it takes for the recent blockhash to age out (ie. become old enough to fail the getFeeCalculatorForBlockhash check).

@jordaaash
Copy link
Collaborator

Yeah, true. We can check it up front without signing just by hashing it. We could also just sign it and use the signature if it passes verification, and then do the real validation afterward.

@jordaaash
Copy link
Collaborator

how about we use that hash of the transaction itself as a lock for Octane?

Implemented in #3:
https://github.com/solana-labs/octane/blob/8479af3e78a4a613006405c535ba052aaac5b2f7/src/api/transfer.ts#L17-L19

Let me know what you think!

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

No branches or pull requests

2 participants