Skip to content

Commit

Permalink
moves change notes construction inside of build function of `Propos…
Browse files Browse the repository at this point in the history
…edTransaction`. Makes it so no new method is required for build layer (#4572)
  • Loading branch information
jowparks authored Jan 22, 2024
1 parent 54db84e commit cd02987
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 8 deletions.
2 changes: 1 addition & 1 deletion ironfish-rust-nodejs/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ export class Transaction {
* aka: self.value_balance - intended_transaction_fee - change = 0
*/
post(spenderHexKey: string, changeGoesTo: string | undefined | null, intendedTransactionFee: bigint): Buffer
build(proofGenerationKeyStr: string, viewKeyStr: string, outgoingViewKeyStr: string, publicAddressStr: string, intendedTransactionFee: bigint): Buffer
build(proofGenerationKeyStr: string, viewKeyStr: string, outgoingViewKeyStr: string, publicAddressStr: string, intendedTransactionFee: bigint, changeGoesTo?: string | undefined | null): Buffer
setExpiration(sequence: number): void
}
export class FoundBlockResult {
Expand Down
6 changes: 6 additions & 0 deletions ironfish-rust-nodejs/src/structs/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,13 +321,18 @@ impl NativeTransaction {
outgoing_view_key_str: String,
public_address_str: String,
intended_transaction_fee: BigInt,
change_goes_to: Option<String>,
) -> Result<Buffer> {
let view_key = ViewKey::from_hex(&view_key_str).map_err(to_napi_err)?;
let outgoing_view_key =
OutgoingViewKey::from_hex(&outgoing_view_key_str).map_err(to_napi_err)?;
let public_address = PublicAddress::from_hex(&public_address_str).map_err(to_napi_err)?;
let proof_generation_key = ProofGenerationKey::from_hex(&proof_generation_key_str)
.map_err(|_| to_napi_err("PublicKeyPackage hex to bytes failed"))?;
let change_address = match change_goes_to {
Some(address) => Some(PublicAddress::from_hex(&address).map_err(to_napi_err)?),
None => None,
};
let unsigned_transaction = self
.transaction
.build(
Expand All @@ -336,6 +341,7 @@ impl NativeTransaction {
outgoing_view_key,
public_address,
intended_transaction_fee.get_i64().0,
change_address,
)
.map_err(to_napi_err)?;

Expand Down
13 changes: 10 additions & 3 deletions ironfish-rust/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ impl ProposedTransaction {
Ok(())
}

pub fn add_change_notes(
fn add_change_notes(
&mut self,
change_goes_to: Option<PublicAddress>,
public_address: PublicAddress,
Expand Down Expand Up @@ -223,7 +223,6 @@ impl ProposedTransaction {
change_notes.push(change_note);
}
}

for change_note in change_notes {
self.add_output(change_note)?;
}
Expand All @@ -237,7 +236,14 @@ impl ProposedTransaction {
outgoing_view_key: OutgoingViewKey,
public_address: PublicAddress,
intended_transaction_fee: i64,
change_goes_to: Option<PublicAddress>,
) -> Result<UnsignedTransaction, IronfishError> {
// skip adding change notes if this is special case of a miners fee transaction
let is_miners_fee = self.outputs.iter().any(|output| output.get_is_miners_fee());
if !is_miners_fee {
self.add_change_notes(change_goes_to, public_address, intended_transaction_fee)?;
}

// The public key after randomization has been applied. This is used
// during signature verification. Referred to as `rk` in the literature
// Calculated from the authorizing key and the public_key_randomness.
Expand Down Expand Up @@ -330,14 +336,14 @@ impl ProposedTransaction {
let public_address = spender_key.public_address();

let i64_fee = i64::try_from(intended_transaction_fee)?;
self.add_change_notes(change_goes_to, public_address, i64_fee)?;

let unsigned = self.build(
spender_key.sapling_proof_generation_key(),
spender_key.view_key().clone(),
spender_key.outgoing_view_key().clone(),
public_address,
i64_fee,
change_goes_to,
)?;
unsigned.sign(spender_key)
}
Expand Down Expand Up @@ -378,6 +384,7 @@ impl ProposedTransaction {
spender_key.outgoing_view_key().clone(),
spender_key.public_address(),
*self.value_balances.fee(),
None,
)?;
unsigned.sign(spender_key)
}
Expand Down
4 changes: 4 additions & 0 deletions ironfish-rust/src/transaction/outputs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ impl OutputBuilder {
self.is_miners_fee = true;
}

pub(crate) fn get_is_miners_fee(&self) -> bool {
self.is_miners_fee
}

/// Get the value_commitment from this proof as an edwards Point.
///
/// This integrates the value and randomness into a single point, using an
Expand Down
6 changes: 2 additions & 4 deletions ironfish-rust/src/transaction/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,8 @@ fn test_proposed_transaction_build() {
transaction.add_output(out_note).unwrap();
assert_eq!(transaction.outputs.len(), 1);

let public_address = spender_key.public_address();
let public_address: crate::PublicAddress = spender_key.public_address();
let intended_fee = 1;
transaction
.add_change_notes(Some(public_address), public_address, intended_fee)
.expect("should be able to add change notes");

let unsigned_transaction = transaction
.build(
Expand All @@ -240,6 +237,7 @@ fn test_proposed_transaction_build() {
spender_key.outgoing_view_key().clone(),
spender_key.public_address(),
intended_fee,
Some(public_address),
)
.expect("should be able to build unsigned transaction");

Expand Down

0 comments on commit cd02987

Please sign in to comment.