Purge transactions

[GabrielMartinezRodriguez]

Linked Issues

• closes https://linear.app/happychain/issue/HAPPY-113/txmanager-we-should-purge-finalized-transactions-after-a-delay

Description

This PR implements a mechanism that removes finalized transactions after a period without changes. The time interval for removing the transaction is configurable, with a default of 2 minutes

Toggle Checklist

Checklist

Basics

• B1. I have applied the proper label & proper branch name (e.g. norswap/build-system-caching).
• B2. This PR is not so big that it should be split & addresses only one concern.
• B3. The PR targets the lowest branch it can (ideally master).

Reminder: PR review guidelines

Correctness

• C1. Builds and passes tests.
• C2. The code is properly parameterized & compatible with different environments (e.g. local,
  testnet, mainnet, standalone wallet, ...).
• C3. I have manually tested my changes & connected features.

This PR has been tested using the randomness service on localhost, interacting with the Anvil blockchain

• C4. I have performed a thorough self-review of my code after submitting the PR,
  and have updated the code & comments accordingly.

Architecture & Documentation

• D1. I made it easy to reason locally about the code, by (1) using proper abstraction boundaries,
  (2) commenting these boundaries correctly, (3) adding inline comments for context when needed.
• D2. All public-facing APIs & meaningful (non-local) internal APIs are properly documented in code
  comments.
• D3. If appropriate, the general architecture of the code is documented in a code comment or
  in a Markdown document.

[GabrielMartinezRodriguez]

• Add flush mechanism #289 : 2 dependent PRs (#297 , #298 )
• Allow run multiple txm simultaneously within the same service #285
• Added docs for txm #272
• Mutex in tx monitor #264
• Purge transactions #232 👈 (View in Graphite)
• Txm hooks #229
• Use kysely instead of mikro-orm #250
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[cloudflare-workers-and-pages]

Deploying happychain with    Cloudflare Pages

Latest commit:	ef383c3
Status:	⚡️  Build in progress...

View logs

[linear]

HAPPY-113 txmanager: We should purge finalized transactions after a delay

We should implement a system for purging transactions after a delay. The delay could be configurable, or perhaps occur after block finalization, considering a future system that handles possible forks

[not-reed]

should this not be some timestamp column?

[GabrielMartinezRodriguez]

SQLite doesn't have a timestamp type, so what I'm doing is saving the timestamp in a numeric format. https://www.sqlite.org/datatype3.html

[not-reed]

the more i use sqlite the more my lovehate relationship with it grows 😄

I guess i would still be inclined to use a date or datetime https://www.sqlite.org/datatype3.html#affinity_name_examples here in the migration, then if we use something other than sqlite it'll still make sense 🤔 maybe this will strictly always be sqlite though 🤷

just sharing thoughts, don't have to change

[GabrielMartinezRodriguez]

If I use the datetime type, I get this error: invalid column data type "DATETIME"

[norswap]

This is probably the explanation: https://stackoverflow.com/questions/17227110/how-do-datetime-values-work-in-sqlite

SQLite allows anything (including DATETIME) as declared type of a column. Based on that, it gives that column an affinity with a storage class (it even has the example of how this works for DATETIME in the documentation). That affinity is more like a hint, as each entry the column can actually have a different storage class. A storage class is still a step weaker than a type and can be backed by multiple types. So yes, you can use DATETIME. No, it does not actually support it as a type or storage class. Yes, the documentation actually contains the word "DATETIME".

So there's probably something here that validates that the type is one of the "standard" types.

In any case, DATETYPE gives NUMERIC affinity which is probably not what we want to store timestamps.

Let's add a comment here though to explain this. Somethiing like "SQLite does not have native time types. The SQL interface allows arbitrary type names including "DATE" and "DATETIME", but this is invalid in this API, and results in "NUMERIC" affinity instead of "INTEGER" affinity, which is the one we want here."

[not-reed]

Suggested change

	this.notFinalizedTransactions = this.notFinalizedTransactions = this.notFinalizedTransactions.filter(
	this.notFinalizedTransactions = this.notFinalizedTransactions.filter(

🧐 not sure what happened here 😁

[not-reed]

and there 🤔

Suggested change

	this.notFinalizedTransactions = this.notFinalizedTransactions = this.notFinalizedTransactions.filter(
	this.notFinalizedTransactions = this.notFinalizedTransactions.filter(

[not-reed]

the more i use sqlite the more my lovehate relationship with it grows 😄

I guess i would still be inclined to use a date or datetime https://www.sqlite.org/datatype3.html#affinity_name_examples here in the migration, then if we use something other than sqlite it'll still make sense 🤔 maybe this will strictly always be sqlite though 🤷

just sharing thoughts, don't have to change

[norswap]

Has this been tested (even if just manually)? If not, we should probably make sure it works.

[norswap]

Should we enable setting this to 0 to disable purging?

[GabrielMartinezRodriguez]

It's a good idea. I just implemented it

[norswap]

This is probably the explanation: https://stackoverflow.com/questions/17227110/how-do-datetime-values-work-in-sqlite

SQLite allows anything (including DATETIME) as declared type of a column. Based on that, it gives that column an affinity with a storage class (it even has the example of how this works for DATETIME in the documentation). That affinity is more like a hint, as each entry the column can actually have a different storage class. A storage class is still a step weaker than a type and can be backed by multiple types. So yes, you can use DATETIME. No, it does not actually support it as a type or storage class. Yes, the documentation actually contains the word "DATETIME".

So there's probably something here that validates that the type is one of the "standard" types.

In any case, DATETYPE gives NUMERIC affinity which is probably not what we want to store timestamps.

Let's add a comment here though to explain this. Somethiing like "SQLite does not have native time types. The SQL interface allows arbitrary type names including "DATE" and "DATETIME", but this is invalid in this API, and results in "NUMERIC" affinity instead of "INTEGER" affinity, which is the one we want here."

[GabrielMartinezRodriguez]

Has this been tested (even if just manually)? If not, we should probably make sure it works.

Yes, I have tested it