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

bugs related to retrying an aborted transfer #287

Open
zoedberg opened this issue Nov 6, 2024 · 5 comments · May be fixed by #288
Open

bugs related to retrying an aborted transfer #287

zoedberg opened this issue Nov 6, 2024 · 5 comments · May be fixed by #288
Assignees

Comments

@zoedberg
Copy link
Contributor

zoedberg commented Nov 6, 2024

While using the RGB sandbox @nicbus discovered some bugs related to retrying an aborted transfer.

To simplify and minimize the scope of the debug we tried to reproduce the same bugs in rgb-tests: RGB-WG/rgb-tests#23

The PR introduces 2 new tests that recreate a very similar scenario, where the only relevant change is the call to update_witnesses(), and they show different bugs. In detail:

  • same_transfer_twice_no_update_witnesses:
    • the 2nd consignment that the sender creates has duplicated bundles spending the same input
    • the receiver considers the 2nd consignment as valid
  • same_transfer_twice_update_witnesses:
    • the sender fails to recreate an identical transfer twice

For context, the bugs happen only:

  • on branches ready for beta 9 (on beta 8 the sender constructs a valid consignement)
  • if the receiver asks to receive on a blinded UTXO
  • (only for same_transfer_twice_no_update_witnesses) if we call resolver.add_terminals
@dr-orlovsky
Copy link
Member

  • same_transfer_twice_no_update_witnesses:
    • the 2nd consignment that the sender creates has duplicated bundles spending the same input
    • the receiver considers the 2nd consignment as valid

Should we be considering this to be a bug? The idea I always had is that all alternative bundles should always be accepted such that if a re-org happens lately, the funds are not lost.

So, what makes a valid consignment is the fact that it contains at least one valid and mined bundle - but it can contain more and that should be ok.

  • same_transfer_twice_update_witnesses:
    • the sender fails to recreate an identical transfer twice

This seems to be an issue and caused by the fact that we consider seals assigned to Tentative (i.e. mempool) witness transactions as spent... Still, this makes unclear how RBF than works...

@zoedberg
Copy link
Contributor Author

Should we be considering this to be a bug? The idea I always had is that all alternative bundles should always be accepted such that if a re-org happens lately, the funds are not lost.

So, what makes a valid consignment is the fact that it contains at least one valid and mined bundle - but it can contain more and that should be ok.

It could be confusing but I agree in general this makes sense.
But I think we agree that only one of those duplicated allocations should be spendable, right?
If so, please check how I've changed the test (RGB-WG/rgb-tests@0c859f5): I've commented the allocation check and added a send, from the wallet that sees the duplicated allocations (i.e. wlt_2), of 200 (i.e. 2x100) assets, and the send works. This seems an important bug to me. I've also added a log of what wlt_1 sees and it reports 1900 + 200 assets as owned, which is more than the issued amount (i.e. 2000).

This seems to be an issue and caused by the fact that we consider seals assigned to Tentative (i.e. mempool) witness transactions as spent... Still, this makes unclear how RBF than works...

If you mean how the rbf_transfer test works, the only relevant difference I've found is that in rbf_transfer both the TXs are broadcasted (but not mined) while in the other two new tests we broadcast only the second TX.

@dr-orlovsky

@dr-orlovsky
Copy link
Member

dr-orlovsky commented Jan 6, 2025

Intermediary report.

My investigation shows that with pay-to-witness a two distinct transitions and bundles are created, since a different blindings are generated for the assignments. This results in a situation when instead of 1 bundle and 2 witness transactions (which work well) we have 2 contradicting bundles and witness transactions.

This doesn't happen with pay-to-utxo, since a blinding is fixed and is provided by the beneficiary in a secret seal. In this case we have just 1 bundle (with 2 different witness transactions) and everything works well.

Investigating further why 2 alternative bundles doesn't work as expected

@dr-orlovsky
Copy link
Member

Ok, now the real bug is the fact that in v0.11.0-beta.9 (as well as in all other pre-v0.12 versions) there is no state filtering basing on the witness transaction status (tentative or not); and all alternative states are seen as valid. This filtering is absent since ContractIface has no access to the witness status (it just knows the witness transaction ids, not their mining information).

I will try to add additional parameter to the ContractIface::extract_state* methods to filter state basing on the witness status and will see whether it is possible without big re-writes of the business logic flow.

@dr-orlovsky
Copy link
Member

I will try to add additional parameter to the ContractIface::extract_state* methods to filter state basing on the witness status and will see whether it is possible without big re-writes of the business logic flow.

This was possible and quite simple. The fix is in #288

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In review
Development

Successfully merging a pull request may close this issue.

2 participants