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/doublespend in optimistic #227

Merged
merged 3 commits into from
Sep 13, 2024
Merged

Fix/doublespend in optimistic #227

merged 3 commits into from
Sep 13, 2024

Conversation

AllFi
Copy link

@AllFi AllFi commented Sep 10, 2024

This PR more or less replicates the behavior of #226 but replaces optimistic indexes with timestamps.

The general idea is to maintain a cache of pending transactions. These pending transactions are merged with already indexed transactions in response to a /transactions/v2 request. The merge process is straightforward: it removes duplicates between the indexed and pending transactions based on commitment and adds the remaining pending transactions, sorted by timestamp, up to the specified limit.

There are three events that trigger the removal of pending transactions:

  • The sent transaction fails.
  • The transaction is indexed by the indexer.
  • The transaction has been in the cache for too long (1 day), though ideally, this situation should never occur.

One consequence of this PR is that pendingDeltaIndex in the /info response is now less accurate. However, this shouldn't be an issue, as it is only used to estimate the limit value for the /transactions/v2 request.

@AllFi AllFi marked this pull request as ready for review September 10, 2024 12:47
@AllFi AllFi requested review from EvgenKor and r0wdy1 September 10, 2024 12:48
Copy link

@EvgenKor EvgenKor left a comment

Choose a reason for hiding this comment

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

lgtm

@r0wdy1 r0wdy1 merged commit a662ce3 into devel Sep 13, 2024
1 check passed
@r0wdy1 r0wdy1 deleted the fix/doublespend_in_optimistic branch September 13, 2024 14:33
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