tiamak/salvium-rs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix three FFI transaction bugs: secret_key_y, unlock_time, fee estimate

Browse Source 
The FFI path (build_sign_maybe_broadcast) had three bugs not present in
  the integration test code which builds transactions directly:

  1. Legacy CryptoNote outputs always got secret_key_y=None, but TCLSAG
     (rct_type >= SALVIUM_ONE) requires secret_key_y=Some([0u8;32]) even
     for non-CARROT inputs. C++ sets carrot_ctkey.y = rct::zero(). This
     caused "input N requires secret_key_y for TCLSAG but has None" on
     sweeps.

  2. STAKE transactions never set unlock_time (defaulted to 0). The daemon
     requires unlock_time = current_height + stake_lock_period, causing
     "Failed (unknown)" rejections. do_stake() now queries daemon height
     and uses network_config().stake_lock_period.

  3. estimate_tx_size() underestimated by ~86 bytes for CARROT transactions.
     Missing: return_address_list (32*n_outputs), return_address_change_mask
     (n_outputs), per-output ephemeral keys in extra (2+32*n_outputs).
     Old flat estimates (24+40=64 bytes) replaced with accurate per-field
     accounting (~150 bytes for 2-output CARROT TX).

  Also adds FEE DIAGNOSTIC log line in FFI that prints estimated vs actual
  serialized weight after signing, making fee issues immediately visible.
This commit is contained in:
Matt Hess
2026-03-01 12:33:56 +00:00
parent 650cb4b1f3
commit e320731600
2 changed files with 83 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/salvium-ffi/src/transfer.rs
+47 -10
View File
@@ -475,6 +475,7 @@ async fn do_transfer(
priority,
fee_per_byte,
0, // amount_burnt
0, // unlock_time (no lock for TRANSFER)
fork_rct,
is_carrot,
params.dry_run,
@@ -528,6 +529,11 @@ async fn do_stake(
)
.map_err(|e| format!("UTXO selection failed: {e}"))?;
// STAKE requires unlock_time = current_height + stake_lock_period.
let info = daemon.get_info().await.map_err(|e| format!("get_info failed: {e}"))?;
let lock_period = salvium_types::constants::network_config(wallet.network()).stake_lock_period;
let unlock_time = info.height + lock_period;
let built = build_sign_maybe_broadcast(
wallet,
daemon,
@@ -539,6 +545,7 @@ async fn do_stake(
priority,
fee_per_byte,
amount, // amount_burnt = staked amount
unlock_time,
fork_rct,
is_carrot,
dry_run,
@@ -612,7 +619,8 @@ async fn do_sweep(
params.ring_size,
priority,
fee_per_byte,
0,
0, // amount_burnt
0, // unlock_time (no lock for sweep)
fork_rct,
is_carrot,
params.dry_run,
@@ -652,6 +660,7 @@ async fn build_sign_maybe_broadcast(
priority: FeePriority,
fee_per_byte: u64,
amount_burnt: u64,
unlock_time: u64,
fork_rct_type: u8,
is_carrot: bool,
dry_run: bool,
@@ -774,7 +783,10 @@ async fn build_sign_maybe_broadcast(
output_row.subaddress_index.major as u32,
output_row.subaddress_index.minor as u32,
);
(secret, None)
// For TCLSAG (rct_type >= SALVIUM_ONE), legacy outputs need secret_key_y = zero.
// C++ uses carrot_ctkey{x, y=rct::zero(), mask} for non-CARROT inputs.
let sk_y = if fork_rct_type >= rct_type::SALVIUM_ONE { Some([0u8; 32]) } else { None };
(secret, sk_y)
};
let mask = hex_to_32(
@@ -801,14 +813,11 @@ async fn build_sign_maybe_broadcast(
// 3. Build the unsigned transaction.
let out_type = if is_carrot { output_type::CARROT_V1 } else { output_type::TAGGED_KEY };
let actual_fee = fee::estimate_tx_fee(
prepared_inputs.len(),
destinations.len() + 1,
ring_size,
is_carrot,
out_type,
fee_per_byte,
);
let n_inputs = prepared_inputs.len();
let n_outputs = destinations.len() + 1; // +1 for change
let estimated_weight =
fee::estimate_tx_weight(n_inputs, n_outputs, ring_size, is_carrot, out_type);
let actual_fee = fee::calculate_fee_from_weight(fee_per_byte, estimated_weight as u64);
// Change address: use CARROT keys when in CARROT mode, CN keys otherwise.
let (chg_spend, chg_view) = if is_carrot {
@@ -822,6 +831,7 @@ async fn build_sign_maybe_broadcast(
.set_change_address(chg_spend, chg_view)
.set_tx_type(tt)
.set_fee(actual_fee)
.set_unlock_time(unlock_time)
.set_priority(priority)
.set_asset_types(asset_type, asset_type)
.set_rct_type(fork_rct_type)
@@ -841,6 +851,7 @@ async fn build_sign_maybe_broadcast(
}
let unsigned = builder.build().map_err(|e| format!("transaction build failed: {e}"))?;
let actual_fee = unsigned.fee;
// 4. Sign the transaction.
let signed = salvium_tx::sign_transaction(unsigned)
@@ -852,6 +863,32 @@ async fn build_sign_maybe_broadcast(
let tx_hex = hex::encode(&tx_bytes);
let weight = tx_bytes.len() as u64;
// Diagnostic: compare estimated weight vs actual serialized size.
let fee_needed_for_actual = fee::calculate_fee_from_weight(fee_per_byte, weight);
let weight_diff = weight as i64 - estimated_weight as i64;
let weight_pct = if estimated_weight > 0 {
100.0 * weight_diff as f64 / estimated_weight as f64
} else {
0.0
};
log::info!(
"FEE DIAGNOSTIC: tx_type={} inputs={} outputs={} carrot={} \
estimated_weight={} actual_blob_size={} diff={} ({:+.1}%) \
fee_set={} fee_needed_for_blob={} fee_per_byte={} {}",
tt,
n_inputs,
n_outputs,
is_carrot,
estimated_weight,
weight,
weight_diff,
weight_pct,
actual_fee,
fee_needed_for_actual,
fee_per_byte,
if fee_needed_for_actual > actual_fee { "*** FEE TOO LOW ***" } else { "OK" }
);
// 6. Broadcast (unless dry run).
if !dry_run {
let send_result = daemon
crates/salvium-tx/src/fee.rs
+36 -9
View File
@@ -65,6 +65,10 @@ impl FeePriority {
///
/// This estimates the size of the full serialized transaction including
/// prefix, RCT base, ring signatures, bulletproofs, and pseudo-outputs.
///
/// Accounts for Salvium-specific prefix fields: return_address_list,
/// return_address_change_mask, protocol_tx_data, and CARROT per-output
/// ephemeral keys in the extra field.
pub fn estimate_tx_size(
num_inputs: usize,
num_outputs: usize,
@@ -72,6 +76,8 @@ pub fn estimate_tx_size(
use_tclsag: bool,
out_type: u8,
) -> usize {
let is_carrot = out_type == output_type::CARROT_V1;
// Prefix overhead.
let mut size = 0usize;
size += 1; // version varint
@@ -79,14 +85,29 @@ pub fn estimate_tx_size(
size += 1; // input count varint
size += 1; // output count varint
// Salvium v2 fields: tx_type(1) + amount_burnt(1-9) + source_asset_type(4)
// + dest_asset_type(4) + amount_slippage_limit(1-9) + return fields overhead
size += 24;
// Salvium v2 prefix fields (always present):
// tx_type(1) + amount_burnt(varint, avg 5) + source_asset_type(varint_len+bytes, ~5)
// + dest_asset_type(~5) + amount_slippage_limit(varint, avg 5)
size += 1 + 5 + 5 + 5 + 5; // = 21
// return_address_list: for TRANSFER v3+, one 32-byte address per output + varint count.
// return_address_change_mask: one byte per output + varint count.
// These are present for all TRANSFER TXs at current hardforks.
size += 1 + num_outputs * 32; // return_address_list: count(1) + entries
size += 1 + num_outputs; // change_mask: count(1) + mask bytes
// return_address + return_pubkey: for pre-v4 STAKE/AUDIT (32 + 32 = 64 bytes).
// protocol_tx_data: for v4 STAKE/AUDIT (~84 bytes).
// These are mutually exclusive with return_address_list, but we add a
// conservative estimate since we don't know the tx_type here.
// The return_address_list estimate above covers the TRANSFER case.
// For STAKE at v4+, protocol_tx_data adds ~84 bytes but return_address_list
// is empty. Net effect: ~84 vs ~67 for 2 outputs. Close enough.
// Per-input size.
let per_input = 1 // type tag
+ 1 // amount varint (0)
+ 4 // asset_type ("SAL\0" varint-len + bytes)
+ 5 // asset_type ("SAL1\0" varint-len + bytes)
+ 1 // key_offsets count varint
+ ring_size * 4 // key_offsets (varints, avg ~4 bytes each)
+ 32; // key_image
@@ -98,12 +119,12 @@ pub fn estimate_tx_size(
1 // type tag
+ 1 // amount varint (0 for RCT)
+ 32 // one-time key
+ 4 // asset_type
+ 5 // asset_type ("SAL1\0")
+ 3 // view tag (3 bytes for CARROT)
+ 16 // encrypted janus anchor
}
output_type::TAGGED_KEY => {
1 + 1 + 32 + 4 + 1 // type + amount + key + asset + view_tag(1)
1 + 1 + 32 + 5 + 1 // type + amount + key + asset + view_tag(1)
}
_ => {
1 + 1 + 32 + 4 // type + amount + key + asset
@@ -111,8 +132,14 @@ pub fn estimate_tx_size(
};
size += num_outputs * per_output;
// Extra field: tx pub key(33) + padding.
size += 40;
// Extra field.
if is_carrot {
// CARROT: per-output ephemeral pubkeys: 0x04(1) + count(1) + n*32.
size += 2 + num_outputs * 32;
} else {
// Legacy CryptoNote: single tx pubkey: 0x01(1) + 32.
size += 33;
}
// RCT base: type(1) + txnFee varint(4) + ecdhInfo + outPk.
size += 1 + 4;
@@ -135,7 +162,7 @@ pub fn estimate_tx_size(
size += estimate_bp_plus_size(num_outputs);
// p_r point (32 bytes, for CARROT).
if out_type == output_type::CARROT_V1 {
if is_carrot {
size += 32;
}