fix: incorrect witness status in state_all method for RBF scenarios

[will-bitlightlabs]

PR Description

Summary

This PR fixes a bug in the state_all method where unbroadcasted RBF (Replace-By-Fee) transactions were incorrectly marked with Mined status instead of their actual Archived status.

Problem Description

In RBF scenarios, when a sender creates multiple transactions (original + replacement) but only broadcasts one of them, the state_all method was returning incorrect witness status for unbroadcasted transactions. Specifically:

• Expected behavior: Unbroadcasted transactions should have Archived status
• Actual behavior: Unbroadcasted transactions were incorrectly showing Mined status
• Impact: This caused confusion in asset state tracking and debugging

Root Cause

The issue was in rgb-std/src/contract.rs where the state collection logic was not properly querying the actual witness status for each witness ID. Instead, it was using a potentially incorrect or hardcoded status value.

Solution

Before (problematic code):

for wid in self.pile.op_witness_ids(addr.opid) {
    state.push(OwnedState {
        addr,
        assignment: Assignment { seal: seal.resolve(wid), data: data.clone() },
        status: since, //  Using incorrect status
    });
}

After (fixed code):

for wid in self.pile.op_witness_ids(addr.opid) {
    let status = self.pile.witness_status(wid); //  Query actual witness status
    state.push(OwnedState {
        addr,
        assignment: Assignment { seal: seal.resolve(wid), data: data.clone() },
        status, //  Use actual witness status
    });
}

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 38.7%. Comparing base (4d06d91) to head (884edb2).

Files with missing lines	Patch %	Lines
src/contract.rs	50.0%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #318     +/-   ##
========================================
- Coverage    38.8%   38.7%   -0.1%     
========================================
  Files          11      11             
  Lines        1733    1734      +1     
========================================
- Hits          672     671      -1     
- Misses       1061    1063      +2     

Flag	Coverage Δ
rust	38.7% <50.0%> (-0.1%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dr-orlovsky]

Not sure this is a correct way.

First, the status of an output in state object is not defined only by the witness status of the last transaction, but by the minimum status of all witness transactions leading in the history. So the original code was correct; with the new version re-orgs won't work.

Second, when a new transaction is created, it gets "tentative" status. When an RBF transaction is created, we have two tentative statues. The wallet has to run sync in order for one of the transactions to get archived and the other mined.

I assume the original "bug" report ignores the sync operation, and thus gets the state ambiguity.

[will-bitlightlabs]

I assume the original "bug" report ignores the sync operation, and thus gets the state ambiguity.

Thank you for the detailed explanation!

I understand your concern about the status calculation logic. However, I want to clarify that test case, it did execute the sync operation before checking state_all.

Even after the sync operation, the unbroadcasted RBF transaction still shows Mined status instead of the expected Archived status. The broadcasted transaction correctly shows Mined.

[dr-orlovsky]

Ok, after more careful consideration I found that your approach is correct: we create an independent state entry for each of the witness variants, thus we need to use a specific witness status, not an aggregated one.

However, it seems there was some other bug on top of the one you have discovered, and I tried to fix it as well in 884edb2

Please check does it works with the test

[will-bitlightlabs]

Ok, after more careful consideration I found that your approach is correct: we create an independent state entry for each of the witness variants, thus we need to use a specific witness status, not an aggregated one.

However, it seems there was some other bug on top of the one you have discovered, and I tried to fix it as well in 884edb2

Please check does it works with the test

I tested the fix in commit 884edb2, but unfortunately there's a new issue now. The state_own method is not returning the correct transaction states anymore - it only shows Genesis asset records instead of the actual transfer states.

Looking at the test output:

• wlt_1.state_own only shows Genesis status instead of the change from the transfer
• wlt_2.state_own shows the received assets correctly with Mined status
• But wlt_1 should show the remaining balance (200) from the transfer, not the original Genesis state (600)

It seems like the fix 884edb2 might have affected how state_own tracks transaction states. The wallet that sent the assets is not properly reflecting the state changes after the transfer.

[dr-orlovsky]

Ok, I think your original PR improves things: indeed we need to use a witness-specific status.

So I am merging it - and the rest of work I moved to the new PR #319

[dr-orlovsky]

ACK: e6c1e09