Problem
signing_pub_key in CommonFields is typed as Option<Cow<'a, str>>, but the XRPL protocol always expects this field to be present — either as a hex public key (signed transaction) or an empty string (multi-signed or unsigned). The current new() constructor already papers over this by wrapping the value: Some(signing_pub_key.unwrap_or("".into())), but consumers using the default/builder pattern can still set it to None, which is never valid on-ledger.
Proposed change
- Change
CommonFields.signing_pub_key from Option<Cow<'a, str>> to Cow<'a, str> with #[serde(default)] so deserialization still works when the field is absent (defaults to "")
- Remove
PreparedTransaction and SignedTransaction wrapper types — they exist only to carry signing_pub_key and txn_signature, both of which are already on CommonFields
- Update all transaction model constructors to pass
"".into() instead of None for signing_pub_key
- Update
is_signed() checks from .signing_pub_key.is_some() to !self.signing_pub_key.is_empty()
- Update the
TransactionBuilder::with_signing_pub_key setter accordingly
Context
Originally proposed in #107 by @LimpidCrypto but that PR is now stale with 22 merge conflicts against main due to subsequent transaction model refactors. This issue captures the same intent for a fresh implementation.
Problem
signing_pub_keyinCommonFieldsis typed asOption<Cow<'a, str>>, but the XRPL protocol always expects this field to be present — either as a hex public key (signed transaction) or an empty string (multi-signed or unsigned). The currentnew()constructor already papers over this by wrapping the value:Some(signing_pub_key.unwrap_or("".into())), but consumers using the default/builder pattern can still set it toNone, which is never valid on-ledger.Proposed change
CommonFields.signing_pub_keyfromOption<Cow<'a, str>>toCow<'a, str>with#[serde(default)]so deserialization still works when the field is absent (defaults to"")PreparedTransactionandSignedTransactionwrapper types — they exist only to carrysigning_pub_keyandtxn_signature, both of which are already onCommonFields"".into()instead ofNoneforsigning_pub_keyis_signed()checks from.signing_pub_key.is_some()to!self.signing_pub_key.is_empty()TransactionBuilder::with_signing_pub_keysetter accordinglyContext
Originally proposed in #107 by @LimpidCrypto but that PR is now stale with 22 merge conflicts against main due to subsequent transaction model refactors. This issue captures the same intent for a fresh implementation.