Skip to content

Commit 363652e

Browse files
sgbettclaude
andcommitted
fix(wallet): address review findings — EF source data, status consistency, finalize_action
1. send_with uses the original signed tx from pending entry (preserves source_satoshis/source_locking_script for EF submission) instead of reconstructing from stored hex which loses that metadata 2. promote_no_send sets action status to 'unproven' (matching the returned SendWithResult status) instead of 'completed' 3. finalize_action now uses the same pending→broadcast→promote/rollback discipline as auto_fund_and_create when a broadcaster is configured Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 8bd13d5 commit 363652e

File tree

2 files changed

+48
-13
lines changed

2 files changed

+48
-13
lines changed

gem/bsv-wallet/lib/bsv/wallet_interface/wallet_client.rb

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1272,24 +1272,33 @@ def broadcast_single_no_send(txid)
12721272
return { txid: txid, status: 'failed', error: 'Pending entry expired or already processed' }
12731273
end
12741274

1275-
tx_hex = @storage.find_transaction(txid)
1276-
return { txid: txid, status: 'failed', error: 'Transaction hex not found in storage' } unless tx_hex
1275+
# Use the original signed transaction from the pending entry — it has
1276+
# source_satoshis and source_locking_script wired on each input, which
1277+
# the broadcaster needs for Extended Format (EF/BRC-30) submission.
1278+
# Reconstructing from stored hex would lose that metadata.
1279+
tx = pending_entry[:tx]
1280+
unless tx
1281+
tx_hex = @storage.find_transaction(txid)
1282+
return { txid: txid, status: 'failed', error: 'Transaction not found' } unless tx_hex
12771283

1278-
tx = BSV::Transaction::Transaction.from_hex(tx_hex)
1284+
tx = BSV::Transaction::Transaction.from_hex(tx_hex)
1285+
end
12791286
promote_no_send(tx, txid, fund_ref, pending_entry)
12801287
rescue StandardError => e
12811288
# Catch unexpected errors outside the broadcast rescue block.
12821289
{ txid: txid, status: 'failed', error: e.message }
12831290
end
12841291

12851292
# Attempts to broadcast and promote a single no_send transaction.
1293+
# Action status is set to 'unproven' (matching TS SDK SendWithResult)
1294+
# because the transaction has been submitted but not yet confirmed.
12861295
def promote_no_send(tx, txid, fund_ref, pending_entry)
12871296
@broadcaster.broadcast(tx)
12881297

12891298
# Broadcast succeeded — promote state and remove from pending indices.
12901299
pending_entry[:locked_outpoints].each { |op| @storage.update_output_state(op, :spent) }
12911300
pending_entry[:change_outpoints].each { |op| @storage.update_output_state(op, :spendable) }
1292-
@storage.update_action_status(txid, 'completed')
1301+
@storage.update_action_status(txid, 'unproven')
12931302
@pending.delete(fund_ref)
12941303
@pending_by_txid.delete(txid)
12951304

@@ -1328,15 +1337,13 @@ def finalize_action(tx, args)
13281337
tx.sign_all if tx.inputs.any?(&:unlocking_script_template)
13291338
txid = tx.txid_hex
13301339

1331-
status = if args.dig(:options, :no_send)
1340+
no_send = args.dig(:options, :no_send)
1341+
delayed = args.dig(:options, :accept_delayed_broadcast)
1342+
1343+
status = if no_send
13321344
'nosend'
1333-
elsif args.dig(:options, :accept_delayed_broadcast)
1334-
warn '[bsv-wallet] accept_delayed_broadcast: true is not yet implemented; ' \
1335-
'falling through to synchronous processing. ' \
1336-
'Background broadcasting is a planned future feature.'
1337-
'unproven'
13381345
else
1339-
'completed'
1346+
'pending'
13401347
end
13411348

13421349
@storage.store_transaction(txid, tx.to_hex)
@@ -1345,7 +1352,35 @@ def finalize_action(tx, args)
13451352

13461353
beef_binary = tx.to_beef
13471354
result = { txid: txid, tx: beef_binary.unpack('C*') }
1348-
result[:no_send_change] = [] if args.dig(:options, :no_send)
1355+
1356+
if no_send
1357+
result[:no_send_change] = []
1358+
elsif broadcast_enabled?
1359+
# Broadcast and promote or rollback — same discipline as auto_fund path.
1360+
broadcast_result = nil
1361+
begin
1362+
broadcast_result = @broadcaster.broadcast(tx)
1363+
@storage.update_action_status(txid, 'completed')
1364+
result[:broadcast_result] = broadcast_result
1365+
result[:broadcast_status] = 'success'
1366+
rescue StandardError => e
1367+
@storage.update_action_status(txid, 'failed')
1368+
result[:broadcast_error] = e.message
1369+
result[:broadcast_status] = broadcast_status_for(e)
1370+
end
1371+
else
1372+
# No broadcaster — promote immediately (backwards compatible).
1373+
final_status = if delayed
1374+
warn '[bsv-wallet] accept_delayed_broadcast: true is not yet implemented; ' \
1375+
'falling through to synchronous processing. ' \
1376+
'Background broadcasting is a planned future feature.'
1377+
'unproven'
1378+
else
1379+
'completed'
1380+
end
1381+
@storage.update_action_status(txid, final_status)
1382+
end
1383+
13491384
result
13501385
end
13511386

gem/bsv-wallet/spec/bsv/wallet_interface/wallet_client_auto_fund_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,7 @@ def build_source_beef(satoshis: 5_000)
891891

892892
all_actions = storage.find_actions({ limit: 100, offset: 0 })
893893
action = all_actions.find { |a| a[:txid] == txid }
894-
expect(action[:status]).to eq('completed')
894+
expect(action[:status].to_s).to eq('unproven')
895895
end
896896

897897
it 'returns failed status for unknown txid' do

0 commit comments

Comments
 (0)