Merge pull request #473 from sgbett/feat/466-beef-ancestry-persistence

fix: persist BEEF ancestry through the wallet lifecycle

Author: sgbett

.claude/drafts/upstream-wallet-toolbox-149-comment.md

@@ -0,0 +1,36 @@
+# Draft comment for bsv-blockchain/wallet-toolbox#149
+
+**Not yet posted.** Review before posting. Attribution note: same reporter
+as upstream issue (sgbett / Simon Bettison), so tone can be direct and
+technical.
+
+---
+
+## Comment body
+
+Hi — just a note that I hit the same issue while porting the wallet to Ruby (sgbett/bsv-ruby-sdk, HLR #466), and the investigation turned up a slightly different shape to what I originally thought when I filed this.
+
+My initial instinct (and the workaround in the fork) was that `internaliseAction` wasn't persisting BEEF ancestors. After going deeper, that turned out not to be quite right — `storeProofsFromBeef` was already walking the full BEEF and storing every ancestor transaction. The storage side was fine.
+
+The real issue was in two other places:
+
+**1. EF serialisation (`toHexEF` / `writeEF`)**
+
+When a transaction is built using inputs from `fromBEEF`, those inputs have their `sourceTransaction` wired up but `sourceSatoshis` and `sourceLockingScript` aren't set as explicit properties on the input object. In the Ruby port, `toEF` wasn't falling back to `input.sourceTransaction.outputs[i]` when those explicit fields were missing — it just raised and the caller silently fell back to broadcasting plain hex. ARC then rejected that hex because the parent was unconfirmed.
+
+I had a look at the TS `writeEF` (Transaction.ts ~L672–713) and it accesses `i.sourceTransaction.outputs[i.sourceOutputIndex]` directly, so it should handle this correctly. The Ruby implementation was the divergent one here — the fix was to derive from `sourceTransaction` when explicit fields aren't set, non-mutating, matching the TS behaviour.
+
+**2. `fromBEEF` ancestry wiring**
+
+The second, subtler issue: `Transaction.fromBEEF` (Ruby's `from_beef`) wasn't routing through `Beef#findAtomicTransaction` (our `find_atomic_transaction`). That method does the extra step of attaching late-bound BUMPs to `FORMAT_RAW_TX` ancestors whose txid appears as a leaf elsewhere in the BUMP tree — a pass that `wireSourceTransactions` alone doesn't cover. So the returned transaction had some ancestors without `merklePath` attached even when the BEEF contained the proof.
+
+The fix was to go through `findAtomicTransaction` / `find_atomic_transaction` rather than the simpler path, so every ancestor in the returned tx has its proof wired before you try to broadcast.
+
+---
+
+The end-to-end symptom is: `internaliseAction` runs fine, the UTXO lands in the wallet, but when you try to spend it in a later `createAction`, the broadcast silently falls back to raw hex (or fails to build EF), and ARC rejects it with "must be valid transaction on chain".
+
+The fix in the Ruby SDK is in [sgbett/bsv-ruby-sdk#473](https://github.com/sgbett/bsv-ruby-sdk/pull/473) if it's useful as a reference.
+
+Whether the TS SDK has the same gap in `fromBEEF` / `fromAtomicBEEF` is worth a look — if `writeEF` already handles the `sourceTransaction` fallback correctly (which it looks like it does), the TS issue might be narrower than "persist ancestors in `internaliseAction`". Happy to dig into it further if that's useful.
+

docs/gems/wallet.md

@@ -148,6 +148,12 @@ wallet.internalize_action({
 })
 ```
 
+The full internalize → create → broadcast round-trip is exercised by the integration
+spec suite. `internalize_action` persists every transaction in the BEEF (not just the
+subject), so ancestor chain data is available when the wallet later builds a new
+transaction spending the internalised UTXO. This ensures `to_ef_hex` can serialise the
+spending transaction correctly for ARC broadcast, even when the parent is unconfirmed.
+
 ## Status meanings
 
 Each action stored by the wallet carries a `status` field that reflects its

docs/sdk/transaction.md

@@ -193,6 +193,20 @@ tx = beef.find_transaction(txid_bytes)
 
 BEEF automatically wires source transactions: inputs that reference other transactions in the bundle will have their `source_transaction` set.
 
+### Extracting a transaction from a BEEF
+
+`Transaction.from_beef` returns the subject transaction with full ancestry wired, including late-bound BUMP attachment for ancestors stored as raw transactions alongside a separately-bundled merkle proof:
+
+```ruby
+tx = BSV::Transaction::Transaction.from_beef(beef_bytes)
+
+# Source data is derived from the wired ancestry — no need to set
+# source_satoshis or source_locking_script explicitly.
+ef_bytes = tx.to_ef_hex  # ready for ARC broadcast
+```
+
+This is the canonical flow for re-broadcasting a received BEEF (e.g. after `internalize_action`). `to_ef_hex` resolves source satoshis and locking scripts directly from `source_transaction.outputs[prev_tx_out_index]` when the explicit fields are not set on an input.
+
 ### Transaction Entries
 
 Each entry in a BEEF bundle has a format:

gem/bsv-sdk/CHANGELOG.md

@@ -5,6 +5,26 @@ All notable changes to the `bsv-sdk` gem are documented here.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
 and this gem adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Fixed
+
+- `Transaction#to_ef` now derives `source_satoshis` and `source_locking_script`
+  from `input.source_transaction.outputs[input.prev_tx_out_index]` when the
+  explicit fields are unset. Previously `to_ef` raised `ArgumentError` on inputs
+  wired via `Transaction.from_beef`, causing `ARC#broadcast` to silently fall back
+  to raw hex — which ARC rejects when parent transactions are unconfirmed.
+  Consumer impact: recently-received UTXOs that previously could not be
+  re-broadcast now can. Matches TS SDK `writeEF` behaviour. (#467, HLR #466)
+- `Transaction.from_beef` and `from_beef_hex` now use `Beef#find_atomic_transaction`
+  to locate the subject transaction, ensuring that `FORMAT_RAW_TX` ancestors whose
+  txid appears as a leaf in a separately-stored BUMP have their `merkle_path`
+  attached. Covers a late-bound BUMP attachment gap not handled by the initial
+  `wire_source_transactions` pass. Matches `Transaction.fromAtomicBEEF` semantics
+  in the TS SDK. (#468, HLR #466)
+
+**Related upstream issue:** [wallet-toolbox#149](https://github.com/bsv-blockchain/wallet-toolbox/issues/149) — same architectural gap in the TS SDK.
+
 ## 0.12.0 — 2026-04-15
 
 ### Added

gem/bsv-sdk/lib/bsv/transaction/transaction.rb

@@ -102,19 +102,29 @@ def to_hex
       # EF embeds source satoshis and source locking scripts in each input,
       # allowing ARC to validate sighashes without fetching parent transactions.
       #
+      # Source data is resolved in priority order:
+      # 1. Explicit +source_satoshis+ / +source_locking_script+ on the input.
+      # 2. Derived from +input.source_transaction.outputs[input.prev_tx_out_index]+.
+      #
+      # Source fields on input objects are never mutated — derivation happens on
+      # each call, so calling +to_ef+ twice produces identical output.
+      #
       # @return [String] raw EF transaction bytes
-      # @raise [ArgumentError] if any input is missing source_satoshis or source_locking_script
+      # @raise [ArgumentError] if any input cannot supply source_satoshis or
+      #   source_locking_script via either mechanism
       def to_ef
         buf = [@version].pack('V')
         buf << "\x00\x00\x00\x00\x00\xEF".b
         buf << VarInt.encode(@inputs.length)
-        @inputs.each do |input|
-          raise ArgumentError, 'inputs must have source_satoshis for EF' if input.source_satoshis.nil?
-          raise ArgumentError, 'inputs must have source_locking_script for EF' if input.source_locking_script.nil?
+        @inputs.each_with_index do |input, idx|
+          source_output = ef_source_output(input, idx)
+
+          satoshis = input.source_satoshis || source_output.satoshis
+          locking_script = input.source_locking_script || source_output.locking_script
 
           buf << input.to_binary
-          buf << [input.source_satoshis].pack('Q<')
-          lock_bytes = input.source_locking_script.to_binary
+          buf << [satoshis].pack('Q<')
+          lock_bytes = locking_script.to_binary
           buf << VarInt.encode(lock_bytes.bytesize)
           buf << lock_bytes
         end
@@ -343,21 +353,35 @@ def to_beef_hex
         to_beef.unpack1('H*')
       end
 
-      # Parse a BEEF binary bundle and return the subject transaction
-      # (the last transaction in the bundle).
+      # Parse a BEEF binary bundle and return the subject transaction with
+      # full ancestry wired, including late-bound BUMP attachment.
+      #
+      # For Atomic BEEFs (BRC-95), the subject transaction is identified by
+      # the embedded subject_txid field. For plain BEEFs, the last transaction
+      # with a raw tx entry is used as the subject.
+      #
+      # Uses +find_atomic_transaction+ so that FORMAT_RAW_TX ancestors whose
+      # txid appears as a leaf in a separately-stored BUMP get their
+      # +merkle_path+ wired correctly — a gap not covered by the initial
+      # +wire_source_transactions+ pass in +Beef.from_binary+.
       #
       # @param data [String] raw BEEF binary
-      # @return [Transaction] the subject transaction with ancestry wired
+      # @return [Transaction, nil] the subject transaction with ancestry wired,
+      #   or nil if the BEEF is empty or contains no raw transaction entries
       def self.from_beef(data)
         beef = Beef.from_binary(data)
-        last_tx_entry = beef.transactions.reverse.find(&:transaction)
-        last_tx_entry&.transaction
+        subject_txid = beef.subject_txid ||
+                       beef.transactions.reverse.find(&:transaction)&.transaction&.txid
+        return nil unless subject_txid
+
+        beef.find_atomic_transaction(subject_txid)
       end
 
       # Parse a BEEF hex string and return the subject transaction.
       #
       # @param hex [String] hex-encoded BEEF
-      # @return [Transaction] the subject transaction with ancestry wired
+      # @return [Transaction, nil] the subject transaction with ancestry wired,
+      #   or nil if the BEEF is empty or contains no raw transaction entries
       def self.from_beef_hex(hex)
         from_beef(BSV::Primitives::Hex.decode(hex, name: 'BEEF hex'))
       end
@@ -683,6 +707,39 @@ def fee(model_or_fee = nil, change_distribution: :equal)
 
       private
 
+      # Resolve the source TransactionOutput for EF serialisation.
+      #
+      # Returns nil when both explicit fields (+source_satoshis+ and
+      # +source_locking_script+) are already set on the input, so no derivation
+      # is required. Otherwise, requires a wired +source_transaction+ and a
+      # valid output at +prev_tx_out_index+.
+      #
+      # @param input [TransactionInput]
+      # @param idx [Integer] input index, used in error messages
+      # @return [TransactionOutput, nil]
+      def ef_source_output(input, idx)
+        return nil if input.source_satoshis && input.source_locking_script
+
+        unless input.source_transaction
+          missing = []
+          missing << 'source_satoshis' unless input.source_satoshis
+          missing << 'source_locking_script' unless input.source_locking_script
+          raise ArgumentError,
+                "input #{idx} is missing #{missing.join(' and ')} and has no wired source_transaction"
+        end
+
+        if input.prev_tx_out_index.nil?
+          raise ArgumentError,
+                "input #{idx} has no prev_tx_out_index — cannot derive EF source data"
+        end
+
+        output = input.source_transaction.outputs[input.prev_tx_out_index]
+        return output if output
+
+        raise ArgumentError,
+              "input #{idx} source_transaction has no output at index #{input.prev_tx_out_index}"
+      end
+
       def verify_input_requirements(tx, input, index)
         tx_id = tx.txid_hex
         raise ArgumentError, "input #{index} of transaction #{tx_id} has no unlocking script" if input.unlocking_script.nil?