RGB state is not updated on blockchain re-orgs or RBFs

[nicbus]

Using this branch of rgb-sandbox I get zero balance after accepting a transfer twice.

Here's what the test run does:

• set up the test environment, the wallets and the asset
• stop esplora, save esplora and wallet 1 (issuer/sender) data
• restart esplora
• execute a transfer between wallet 1 and 2, using a blinded UTXO
• stop esplora, restore saved esplora and wallet 1 data
• execute the same transfer between wallet 1 and 2 again

The 2nd transfer re-uses the same invoice as the 1st one and overrides the expected final balance, as it's supposed to be the same once all is done.

Before the 2nd transfer, the recipient sees one UTXO and the asset balance from the 1st transfer, as expected. The transfer is successful, including the recipient accepting it into the stash.

The problem is that, after the 2nd transfer, the recipient sees no UTXOs and therefore the asset balance is 0. The expected outcome is that the same UTXO and asset balance as before the 2nd transfer should be visible.

This happens with both opret and tapret.

[dr-orlovsky]

1. Doing two transfers for the same invoice results in two distinct transfers, not the same one. The reason is that the witness transaction id will be different, thus anchors are different, change UTXOs are different etc. Why we change the witness txid? Due to randomness in the state assignments and MPC entropy. We can do a deterministic transfers with getting the same witness transaction and consignment if we pay the same invoice (but do not mine the first witness tx), but we do not have that API full exposed yet.

2. Thus, for now, the expected behavior is:

• if you mine the witness tx for the first transfers, you would have two independent transfers paying the same external UTXO twice, and you would two two payments
• if you do not mine the witness tx, the receiver should see 0 balance (since the transfer is unconfirmed)

You haven't specified whether the witness tx for the transfers is mined. If it is, than we have the expected behavior

[nicbus]

1. Yep, sorry for the imprecision, I meant the same "logical" transfer, not the exact same one. Some details of the transaction are different the second time but what remains the same are the allocation being spent, the amounts and the UTXO where the new allocation is created. What I think is relevant in this scenario is that, when the 2nd transfer happens, the transaction anchoring the 1st transfer has disappeared from the blockchain.
2. I'm not sure I got what you meant here, but it allowed me to find a bug in the sandbox run, which I have now addressed.

In this rgb-sandbox branch I create the recipient UTXO before saving the blockchain state, which lets the wallet see it after restoring the previous state. In addition to that, I no more save wallet 1 (issuer) data and sync both wallets before attempting the 2nd transfer. With this updated scenario, I expect:

• the sender to see the total issued amount as balance before transfer 2 (and it happens)
• the recipient to see a balance of 0 before transfer 2 (it instead sees the amount from the 1st transfer)
• the sender to see the same balance as after transfer 1 once transfer 2 is complete (and it happens)
• the recipient to see the same balance as after transfer 1 once transfer 2 is complete (it instead sees double that)

To further clarify, I added a 3rd transfer, where the recipient (of the first 2 transfers) tries to spend both allocations (of which one should not be visible) to a new wallet. I expect the validation to fail and indeed when the new wallet tries to validate the transfer it fails with:

Unknown public witnesses:
- bc:cf4179cca2d6e52aead9a588827399cd32a13188e8ca1e4993a01b34bb580d5d
Validation failures:
- witness bc:cf4179cca2d6e52aead9a588827399cd32a13188e8ca1e4993a01b34bb580d5d is not known to the transaction resolver.

[dr-orlovsky]

I tried to read through it 3 times and got lost each time.

Can we distil a specific bug out of this or this is the actual task we still need to do?

[nicbus]

Can we distil a specific bug out of this or this is the actual task we still need to do?

IMO the bug here is that the wallet doesn't update his view of the contract allocations based on changes in the blockchain.

Specifically, if a witness tx disappears (e.g. because of a reorg) then the wallet will still show the allocation that was anchored to the disappeared tx. Trying to spend that allocation will result in a validation error.

The solution here might be to add the possibility to remove/hide a transition (that is, the wallet should not show the resulting allocations anymore), or the possibility to check all transitions and remove/hide the ones that don't have a valid anchor anymore.

[dr-orlovsky]

The whole wallet functionality here is in a command-line tool. Thus, it can't track the blockchain state unless said so. Did you use --sync which is specifically designed for this purpose?

[nicbus]

Did you use --sync which is specifically designed for this purpose?

Yes, the rgb-sandbox script syncs both sender and recipient wallet after restoring the chain data, using the utxos command with the --sync option.

[nicbus]

Tested with an updated rgb-sandbox branch that uses the current master branch of rgb-wallet and the issue is still there.

[dr-orlovsky]

Can you please re-test with the all-new and shiny v0.11?

[nicbus]

Re-tested on branch v0.11 (commit 7aa7361) plus a cargo update to include all the latest fixes and the issue is still there.

The recipient wallet still sees no UTXOs at the end of the 2nd transfer and its asset balance is thus 0.

[zoedberg]

I've added a test called same_transfer_twice in the integration tests PR, which proves that doing an RBF of a colored TX is not possible. By reading and launching the test you can see that the sender first creates a TX to send some RGB assets and broadcasts this TX. The TX doesn't get mined and he tries to recreate it with higher fees but the call to the pay method fails, saying Composition(Construction(NoInputs)). This highlights that the sender has no way to do an RBF, because the sender wallet registers the transfer as finalized as soon as the TX get broadcasted.
I think this is not a desired behavior because it could lead to loss of RGB funds in case a colored TX with too low fees gets broadcasted. @dr-orlovsky which solution do you see for this issue?

[nicbus]

Some updates from rgb-sandbox:

• I have re-tested the original issue on beta 6 and I can reproduce it
• I have also tested an RBF scenario very similar to the one in the same_transfer_twice test zoedberg mentioned and I can reproduce it as well (Error: impossible to construct transaction having no inputs. when calling transfer the 2nd time)

[dr-orlovsky]

@dr-orlovsky which solution do you see for this issue?

Need to think.. Thank you for figuring out the scenario...

[dr-orlovsky]

Ok, the problem is in bp-wallet: BP Wallet removes any spent output from the list of UTXOs, even while tx is not yet mined (in mempool). Opened BP-WG/bp-wallet#48

[dr-orlovsky]

With BP-WG/bp-wallet#49 the test now passes and RBFs work.

My changes for integration tests can be found in https://github.com/zoedberg/rgb-integration-tests/pull/1/files

[zoedberg]

With BP-WG/bp-wallet#49 the test now passes and RBFs work.

I confirm test now passes, PR ACKed

My changes for integration tests can be found in https://github.com/zoedberg/rgb-integration-tests/pull/1/files

This contains changes that I already did locally, sorry, next time I'll push updates everytime RGB master branches get updates

[dr-orlovsky]

This contains changes that I already did locally, sorry, next time I'll push updates everytime RGB master branches get updates

I did the PR to showcase (1) how to use new wallet apis and (2) to ensure the test now passes. Feel free to close it. Just make sure that we use the new wallet apis the same way (like you do not need to call any store/save methods anymore since everything happens automatically now)

[nicbus]

I can confirm the RBF issue is not showing up anymore also in rgb-sandbox.
The reorg version of the issue is still there, though.

@dr-orlovsky please re-open this issue so we keep tracking the other half of the problem.

[dr-orlovsky]

How I can reproduce the re-org issue to investigate it?

[nicbus]

You can reproduce it using this rgb-sandbox branch, by executing:

./demo.sh -v -s 124

The idea is the same as in the original issue report, but using an updated rgb-wallet and including the fixes done to the original sandbox branches.

Here's what the test scenario now does:

• set up the test environment and 2 opret wallets
• issue a NIA asset on wallet_0, export and import it into wallet_1
• generate a UTXO for wallet_0
• stop the services, then save bitcoind and indexer data
• restart the services
• execute a transfer between wallet_0 and wallet_1 (100 assets), using a blinded invoice
• stop the services, then restore the previously saved bitcoind and indexer data
• sync both wallets
• execute the same transfer between wallet_0 and wallet_1 again, re-using the same invoice

I expect the balance for wallet_1 to be 100 (the amount transferred) both after the 1st and 2nd transfer, as the chain status has been restored to the initial state (no transfer) between the two transfers. What I get instead is that wallet_1 sees a balance of 200 after the 2nd transfer or, in other words, it still sees the amount from the 1st transfer even though the witness tx is no more included in the chain.

[dr-orlovsky]

From what I see after running ./demo.sh -v -s 124 is the correct balance. The test fails however, but using command rgb-wallet/bin/rgb -n regtest --electrum=localhost:50001 -d data1 state -w wallet_1 'rgb:IcRdOwGk-VCG1f3z-KQyS9i9-g$ktnl6-ZtRrkCy-TNlAaXM' RGB20Fixed I see that wallet_1 contains correct information:

Loading descriptor from wallet wallet_1 ... success

Global:
  spec := (ticker=("USDT"), name=("USD Tether"), details=~, precision=0)
  terms := (text=("demo NIA asset"), media=~)
  issuedSupply := (2000)

Owned:
  assetOwner:
    value=100, utxo=bc:opret1st:5654929c20b84d15298042a96980e419fafe9471313d9a07f4ccab5f8e91cc68:1, witness=bc:2229024bcffb633d2a08cb5738fa79118ee1bcd4374f2e870145132fbd88b7c7 

[nicbus]

My bad, I inadvertently dropped a fix while preparing the branch for publication.

The state that you're seeing is the one before the 2nd transfer. The test stops because of a wrong check (the computed final balance doesn't match the one provided at command invocation).

I have now updated the branch, correctly specifying 0 as the initial balance for the recipient wallet (wallet_1) and skipping the initial balance check (it would fail because the expected initial balance is 0 but the wallet reports 100). This way the test continues and the 2nd transfer completes. Now the reported failure is the correct one, where at the end of the 2nd transfer the balance should be 100 but is instead 200. Checking the state once the test exits you should see a situation like:

Global:
  spec := (ticker=("USDT"), name=("USD Tether"), details=~, precision=0)
  terms := (text=("demo NIA asset"), media=~)
  issuedSupply := (2000)

Owned:
  assetOwner:
    value=100, utxo=bc:opret1st:2328b825c4ad9d9901324a1b5b89e6866d5f52bbbf88ad214f95650936b97741:1, witness=bc:12cb73c538153315bf994c5c19ef9488b841bd077e1090fe71123d976265142b 
    value=100, utxo=bc:opret1st:2328b825c4ad9d9901324a1b5b89e6866d5f52bbbf88ad214f95650936b97741:1, witness=bc:812dfda69827fabdd79d4b03441d2a9ec7c0edf86c48c5102032887853d5e031 

[dr-orlovsky]

The problems comes to the fact that somehow both witness transaction ids are known to the wallet.

In sandbox, how I can query Bitcoin Core - or better Electrum/Esplora for a specific txid to check whether it is still known to it and what information is returned?

[dr-orlovsky]

Ok, I found the source of the problem.

We do not filter witness transactions - nowhere in the wallet code we attest whether a specific witness transaction is mined or in a mempool.

Witness transactions may not belong to the wallet descriptor and thus are not a part of wallet-basing filtering. They require a specially-designed APIs. I am thinking on the best way of doing it. The main problem - a lot of requests to the indexer which must be handled on update (much more than for usual wallet stuff) - basically we need to check each witness transaction id in the history...

[nicbus]

I have tested the mining branch using the same rgb-sandbox scenario and the outcome is the same as before.

After restoring the chain data (simulating a re-org) and syncing, the recipient wallet still sees the previous allocation, even though the witness transaction is no more available from the blockchain.

I have updated the rgb-sandbox branch to check the recipient balance before re-trying the transfer (previously disabled as it was expected to be wrong) and also added a check for the witness transaction on bitcoind to make sure it isn't returned after the restore (it isn't).

[dr-orlovsky]

I find this log a bit strange:

+ rgb-wallet/bin/rgb -n regtest --electrum=localhost:50001 -d data1 utxos -w wallet_1 --sync
Loading descriptor from wallet wallet_1 ... success
Syncing keychain 0 .......... keychain 1 .......... keychain 9 ........... success
Balance of wpkh([edf4cbdb/84h/1h/0h]tpubDDkwgAKDPZLcUiKbdJiAWUTX3bZApivBQXH3LHRYzjtnBYTxX2TFyxWKDA84qJjGia1SiNE1vmvFrNrSNrk5fw5YWSRxefAo5bQWreDxNJD/<0;1;9>/*)

Height	   Amount, ṩ	Outpoint                                                            
bcrt1qw7pm276yxua6409g0l0g8ukknq527u96mw4kf6	&9/0
105	   100000000	16eb150c7984f1b3859d6f61d08482013b301a78d4bb690b04c6c6564bb303fe:0

It misses the part it must have from here: https://github.com/RGB-WG/rgb/pull/227/files#diff-e5535881c50a9e6adb6c634e4756823e8ca0a816452748017426d349a1ae7504R119-R134 which should read Updating witness information starting from height. Are you sure you updated the rgb to the required PR?

[dr-orlovsky]

Ok, I see the commit is correct:

Installing rgb-wallet v0.11.0-beta.6 (https://github.com/RGB-WG/rgb?branch=mining#bd10bdf6)

Strange that the piece of code I quoted is never reached and executed...

[dr-orlovsky]

Ok, I have finally got it: you provide --sync only for methods which use pure BP and do not initialize Stock. Thus, the witness sync never happens.

[dr-orlovsky]

Ok, adding --sync to rgb state makes witnesses to sync:

diff --git a/demo.sh b/demo.sh
index 06eae60..1e5bf46 100755
--- a/demo.sh
+++ b/demo.sh
@@ -197,7 +197,7 @@ _show_state() {
     schema=${CONTRACT_SCHEMA_MAP[$contract_name]}
     iface=${IFACE_MAP[$schema]}
     _trace "${RGB[@]}" -d "data${wallet_id}" \
-        state -w "$wallet" -a "$contract_id" "$iface"
+        state --sync -w "$wallet" -a "$contract_id" "$iface"
 }

However somehow I see from the debug dumps that the removed after re-org tx now is returned by an indexer as a mempool tx, and thus the state is still invalid. Investigating for indexer bug

[dr-orlovsky]

Ok, the problem was bug in the indexers - they never ever returned that the tx is non-existent and classified all unknown txes as mempool. With this change the tests do work: 8563b58

[nicbus]

I have switched to commit 8563b58 and now call the state command with the --sync option after restoring chain data. With these changes the scenario is now completing successfully with the expected results.

[dr-orlovsky]

So should we close this issue? I expect I can merge the bunch of PRs from mining branches?

[nicbus]

Let's resolve the discussion on telegram before we go ahead and merge those PRs. I'd close the issue once the PRs have been merged.

[dr-orlovsky]

Closing since RGB-WG/rgb-std#247 is merged into the master