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

feat: support an Interaction ID tx tag to prevent duplicated interactions #148

Closed
noomly opened this issue May 24, 2022 · 1 comment
Closed

Comments

@noomly
Copy link
Contributor

noomly commented May 24, 2022

While a bug allowing users to inadvertently run duplicated interactions shouldn't happen in the first place, we can't predict with certitude that it wont ever happen (see #149). Therefore, due to the critical nature of such bugs, I propose a new feature, implemented at the protocol level, that would allow users to optionally and proactively avoid running a unique interaction more than a single time.

This feature would allow a user to provide an Interaction ID (an IID) - that the user would be responsible for generating - as an optional parameter to bundleInteraction. This IID would then be added as a tag to the transaction sent to the Sequencer. When such an IID parameter is given to bundleInteraction, it should be verified before posting the transaction that no other interactions contained in the last X blocks have an IID tag attached to their transaction that it equals to. If another interaction's transaction has an IID equals to the IID given to bundleInteraction, the latter should throw immediately without posting the transaction. The user should be able to configure somewhere the number of previous blocks that should be checked for a duplicate IID.

There is one pain point in implementing this solution that I identified while reading Warp's internals though: RedstoneGatewayInteractionsLoader doesn't use CacheableContractInteractionsLoader (apparently because of #62 (comment)). This means that whenever we'll have to do the check, interactions from the last X blocks will have the be fetched again, which sounds very inefficient. It seems to me that the issue raised by this PR comment should be solved in order to be able the implement the IID feature cleanly. Although I didn't dig into it so I don't really know how much of an hassle it's going to be.

Two more points about the feature:

  • I only mentioned interactions created through bundleInteraction as it's unsure to me if it would be really possible for a regular interaction (without using the Sequencer) as the transactions have a time of confirmation where it's not 100% sure whether they'll end up on chain or not. Which means that we could run two transactions with the same IID in a short time window, where the first transaction would not be confirmed yet and so could be dropped... I'm not sure how we should handle this kind of case.
  • A thought I had while writing: Maybe the check could also happen at the contract evaluation time? Maybe the stage where the check happens (pre-post/pre-evaluation/both) could be configured?

Anyway, I'd be happy to help implement this, if given some insights about the various points I raised above.

@noomly noomly changed the title feat: support a Interaction ID tx tag to prevent duplicated interactions feat: support an Interaction ID tx tag to prevent duplicated interactions May 24, 2022
@ppedziwiatr
Copy link
Contributor

ppedziwiatr commented Sep 22, 2022

We've decided to implement a last_tx feature - i.e. each newly created interaction will have the tx id of the previous transaction from the sequence set in the tags.
This should also allow to protect issues like duplicated transactions - even if such issue would occure, it could be easilly filtered on the SDK level (by the last_tx field).

Implementing this feature requires some major rework of the sequencer implementation - which we're currently actively working on - i.e. we're extracting the sequencer code from gateway to a separate application, rewriting it to Go and refactoring some of its internals (that in effect will allow to effectively implement the last_tx feature) - warp-contracts/gateway#89 (comment)

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