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

docs: add Provider doc section for cacheUtxo #2598

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

Torres-ssf
Copy link
Contributor

@Torres-ssf Torres-ssf added docs Requests pertinent to documentation p1 Medium priority labels Jun 24, 2024
@Torres-ssf Torres-ssf self-assigned this Jun 24, 2024
@Torres-ssf Torres-ssf added this to the 0.x post-launch milestone Jun 24, 2024
@Torres-ssf Torres-ssf marked this pull request as ready for review June 24, 2024 13:26
Copy link
Member

@maschad maschad left a comment

Choose a reason for hiding this comment

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

Nice job on this 🚀 , left some suggestions.

apps/docs/src/guide/provider/provider-options.md Outdated Show resolved Hide resolved
apps/docs/src/guide/provider/provider-options.md Outdated Show resolved Hide resolved
@Torres-ssf Torres-ssf requested a review from maschad June 24, 2024 20:09
Copy link
Contributor

@danielbate danielbate left a comment

Choose a reason for hiding this comment

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

First pass, couple comments and questions.

  1. If I have one utxo and am submitting multiple transactions, cacheUtxo has no effect and in fact we risk the second tx failing but we would be able to fund both txs initially?
  2. Why would you not want to cacheUtxos?

maschad
maschad previously approved these changes Jun 25, 2024
Copy link
Member

@arboleya arboleya left a comment

Choose a reason for hiding this comment

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

I'll think more about this before coming back to it.

At first glance, this flag looks confusing.

apps/docs/src/guide/provider/provider-options.md Outdated Show resolved Hide resolved
Co-authored-by: Anderson Arboleya <[email protected]>
maschad
maschad previously approved these changes Jun 26, 2024
@Torres-ssf Torres-ssf enabled auto-merge (squash) June 27, 2024 16:03
@Torres-ssf
Copy link
Contributor Author

Torres-ssf commented Jun 28, 2024

@danielbate

First pass, couple comments and questions.

  1. If I have one utxo and am submitting multiple transactions, cacheUtxo has no effect, and in fact we risk the second tx failing but we would be able to fund both txs initially?

If you have just one UTXO and you fund and submit 2 TXs simultaneously:

  • without cacheUtxo: The first TX to be processed would work, and the second one would fail with the error related to a non-existent UTXO

  • with cacheUtxos: The first one will be funded normally, but now this UTXO is cached, then when funding the second one an error will thrown related to not having enough balance to fit the target.

When you have just one UTXO, you cannot send multiple TXs without waiting for the first one to complete. It will always fail

  1. Why would you not want to cacheUtxos?

The only reason that comes to mind would be in a situation where your first TX failed, and then your UTXO that was not being spent is now unusable until the cache for it got whipped.

Also, the error message is different. Maybe you have just one UTXO and since the cache is being used by default, and you are completely unaware of it, if you try to send 2 transactions in a row the error message would be not enough balance to fit the target instead of UTXO not existent.

This flag makes it possible for users that own multiple UTXOs to submit multiple TXs in a row, without waiting for each one to be processed, and without running into the error UTXO non-existent.

This error will likely happen, as the TXs are going to be funded automatically, and TX1 and TX2 can be funded with the same UTXO(s). Then, when TX1 is processed, that specific UTXO will be spent, and TX2 will fail.

Copy link
Contributor

github-actions bot commented Jul 1, 2024

Coverage Report:

Lines Branches Functions Statements
79.54%(+0%) 71.37%(+0%) 76.91%(+0%) 79.62%(+0%)
Changed Files:

Coverage values did not change👌.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Requests pertinent to documentation p1 Medium priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add docs related to the optional caching UTXOs logic Review UTXO caching logic
5 participants