removes spender_key from ProposedTransaction (#4476)

* WIP removes spender_key and public_key_randomness from ProposedTransaction

when signing transactions with FROST we will not have a full, valid SaplingKey
to pass as the 'spender_key'. FROST will also require that public key randomness
is shared between signers with randomized parameters used as an input to
signing.

spender_key is only used when posting ProposedTransactions, so these changes
remove it from the struct and instead pass it as an input to each of the post
functions

public_key_randomness is only used in the context of _partial_post, so these
changes remove public_key_randomness from the struct and initialize a randomized
public address from randomness only when needed

* fixes tests

outdated usage of NativeTransaction constructor, post

* passes reference to SaplingKey to post methods

avoids excessive copying to the stack

* restores public_key_randomness to ProposedTransaction

* fixes tests

* fixes benches

* fixes lint

benchmarks/benches/transaction.rs

@@ -24,12 +24,12 @@ pub fn simple(c: &mut Criterion) {
             },
             // Benchmark
             |(key, spend_note, witness, out_note)| {
-                let mut proposed = ProposedTransaction::new(key, TransactionVersion::latest());
+                let mut proposed = ProposedTransaction::new(TransactionVersion::latest());
 
                 proposed.add_spend(spend_note, &witness).unwrap();
                 proposed.add_output(out_note).unwrap();
 
-                let tx = proposed.post(None, 1).unwrap();
+                let tx = proposed.post(&key, None, 1).unwrap();
 
                 assert_eq!(tx.spends().len(), 1);
                 assert_eq!(tx.outputs().len(), 1);
@@ -60,14 +60,14 @@ pub fn all_descriptions(c: &mut Criterion) {
             |(key, spend_note, witness, out_note, asset)| {
                 let asset_value = 10;
 
-                let mut proposed = ProposedTransaction::new(key, TransactionVersion::latest());
+                let mut proposed = ProposedTransaction::new(TransactionVersion::latest());
 
                 proposed.add_spend(spend_note, &witness).unwrap();
                 proposed.add_output(out_note).unwrap();
                 proposed.add_mint(asset, asset_value).unwrap();
                 proposed.add_burn(*asset.id(), asset_value).unwrap();
 
-                let tx = proposed.post(None, 1).unwrap();
+                let tx = proposed.post(&key, None, 1).unwrap();
 
                 assert_eq!(tx.spends().len(), 1);
                 assert_eq!(tx.outputs().len(), 1);
@@ -92,12 +92,12 @@ pub fn verify(c: &mut Criterion) {
 
                 let out_note = Note::new(public_address, 41, "", NATIVE_ASSET, public_address);
 
-                let mut proposed = ProposedTransaction::new(key, TransactionVersion::latest());
+                let mut proposed = ProposedTransaction::new(TransactionVersion::latest());
 
                 proposed.add_spend(spend_note, &witness).unwrap();
                 proposed.add_output(out_note).unwrap();
 
-                proposed.post(None, 1).unwrap()
+                proposed.post(&key, None, 1).unwrap()
             },
             // Benchmark
             |tx| {
@@ -127,12 +127,12 @@ pub fn batch_verify(c: &mut Criterion) {
 
                     let out_note = Note::new(public_address, 41, "", NATIVE_ASSET, public_address);
 
-                    let mut proposed = ProposedTransaction::new(key, TransactionVersion::latest());
+                    let mut proposed = ProposedTransaction::new(TransactionVersion::latest());
 
                     proposed.add_spend(spend_note, &witness).unwrap();
                     proposed.add_output(out_note).unwrap();
 
-                    transactions.push(proposed.post(None, 1).unwrap());
+                    transactions.push(proposed.post(&key, None, 1).unwrap());
                 }
 
                 transactions

ironfish-rust-nodejs/index.d.ts

@@ -180,7 +180,7 @@ export class TransactionPosted {
 }
 export type NativeTransaction = Transaction
 export class Transaction {
-  constructor(spenderHexKey: string, version: number)
+  constructor(version: number)
   /** Create a proof of a new note owned by the recipient in this transaction. */
   output(note: Note): void
   /** Spend the note owned by spender_hex_key at the given witness location. */
@@ -196,12 +196,12 @@ export class Transaction {
    * a miner would not accept such a transaction unless it was explicitly set
    * as the miners fee.
    */
-  post_miners_fee(): Buffer
+  post_miners_fee(spenderHexKey: string): Buffer
   /**
    * Used to generate invalid miners fee transactions for testing. Call
    * post_miners_fee instead in user-facing code.
    */
-  _postMinersFeeUnchecked(): Buffer
+  _postMinersFeeUnchecked(spenderHexKey: string): Buffer
   /**
    * Post the transaction. This performs a bit of validation, and signs
    * the spends with a signature that proves the spends are part of this
@@ -214,7 +214,7 @@ export class Transaction {
    * sum(spends) - sum(outputs) - intended_transaction_fee - change = 0
    * aka: self.value_balance - intended_transaction_fee - change = 0
    */
-  post(changeGoesTo: string | undefined | null, intendedTransactionFee: bigint): Buffer
+  post(spenderHexKey: string, changeGoesTo: string | undefined | null, intendedTransactionFee: bigint): Buffer
   setExpiration(sequence: number): void
 }
 export class FoundBlockResult {

ironfish-rust-nodejs/src/structs/transaction.rs

@@ -167,10 +167,9 @@ pub struct NativeTransaction {
 #[napi]
 impl NativeTransaction {
     #[napi(constructor)]
-    pub fn new(spender_hex_key: String, version: u8) -> Result<Self> {
-        let spender_key = SaplingKey::from_hex(&spender_hex_key).map_err(to_napi_err)?;
+    pub fn new(version: u8) -> Result<Self> {
         let tx_version = version.try_into().map_err(to_napi_err)?;
-        let transaction = ProposedTransaction::new(spender_key, tx_version);
+        let transaction = ProposedTransaction::new(tx_version);
 
         Ok(NativeTransaction { transaction })
     }
@@ -245,8 +244,12 @@ impl NativeTransaction {
     /// a miner would not accept such a transaction unless it was explicitly set
     /// as the miners fee.
     #[napi(js_name = "post_miners_fee")]
-    pub fn post_miners_fee(&mut self) -> Result<Buffer> {
-        let transaction = self.transaction.post_miners_fee().map_err(to_napi_err)?;
+    pub fn post_miners_fee(&mut self, spender_hex_key: String) -> Result<Buffer> {
+        let spender_key = SaplingKey::from_hex(&spender_hex_key).map_err(to_napi_err)?;
+        let transaction = self
+            .transaction
+            .post_miners_fee(&spender_key)
+            .map_err(to_napi_err)?;
 
         let mut vec: Vec<u8> = vec![];
         transaction.write(&mut vec).map_err(to_napi_err)?;
@@ -256,10 +259,11 @@ impl NativeTransaction {
     /// Used to generate invalid miners fee transactions for testing. Call
     /// post_miners_fee instead in user-facing code.
     #[napi(js_name = "_postMinersFeeUnchecked")]
-    pub fn _post_miners_fee_unchecked(&mut self) -> Result<Buffer> {
+    pub fn _post_miners_fee_unchecked(&mut self, spender_hex_key: String) -> Result<Buffer> {
+        let spender_key = SaplingKey::from_hex(&spender_hex_key).map_err(to_napi_err)?;
         let transaction = self
             .transaction
-            .post_miners_fee_unchecked()
+            .post_miners_fee_unchecked(&spender_key)
             .map_err(to_napi_err)?;
 
         let mut vec: Vec<u8> = vec![];
@@ -280,9 +284,12 @@ impl NativeTransaction {
     #[napi]
     pub fn post(
         &mut self,
+        spender_hex_key: String,
         change_goes_to: Option<String>,
         intended_transaction_fee: BigInt,
     ) -> Result<Buffer> {
+        let spender_key = SaplingKey::from_hex(&spender_hex_key).map_err(to_napi_err)?;
+
         let intended_transaction_fee_u64 = intended_transaction_fee.get_u64().1;
 
         let change_key = match change_goes_to {
@@ -292,7 +299,7 @@ impl NativeTransaction {
 
         let posted_transaction = self
             .transaction
-            .post(change_key, intended_transaction_fee_u64)
+            .post(&spender_key, change_key, intended_transaction_fee_u64)
             .map_err(to_napi_err)?;
 
         let mut vec: Vec<u8> = vec![];

ironfish-rust-nodejs/tests/demo.test.slow.ts

@@ -58,11 +58,11 @@ describe('Demonstrate the Sapling API', () => {
   it(`Should create a miner's fee transaction`, () => {
     const key = generateKey()
 
-    const transaction = new Transaction(key.spendingKey, LATEST_TRANSACTION_VERSION)
+    const transaction = new Transaction(LATEST_TRANSACTION_VERSION)
     const note = new Note(key.publicAddress, 20n, 'test', Asset.nativeId(), key.publicAddress)
     transaction.output(note)
 
-    const serializedPostedTransaction = transaction.post_miners_fee()
+    const serializedPostedTransaction = transaction.post_miners_fee(key.spendingKey)
     const postedTransaction = new TransactionPosted(serializedPostedTransaction)
 
     expect(postedTransaction.fee()).toEqual(-20n)
@@ -95,13 +95,13 @@ describe('Demonstrate the Sapling API', () => {
     const key = generateKey()
     const recipientKey = generateKey()
 
-    const minersFeeTransaction = new Transaction(key.spendingKey, LATEST_TRANSACTION_VERSION)
+    const minersFeeTransaction = new Transaction(LATEST_TRANSACTION_VERSION)
     const minersFeeNote = new Note(key.publicAddress, 20n, 'miner', Asset.nativeId(), key.publicAddress)
     minersFeeTransaction.output(minersFeeNote)
 
-    const postedMinersFeeTransaction = new TransactionPosted(minersFeeTransaction.post_miners_fee())
+    const postedMinersFeeTransaction = new TransactionPosted(minersFeeTransaction.post_miners_fee(key.spendingKey))
 
-    const transaction = new Transaction(key.spendingKey, LATEST_TRANSACTION_VERSION)
+    const transaction = new Transaction(LATEST_TRANSACTION_VERSION)
     transaction.setExpiration(10)
     const encryptedNote = new NoteEncrypted(postedMinersFeeTransaction.getNote(0))
     const decryptedNote = Note.deserialize(encryptedNote.decryptNoteForOwner(key.incomingViewKey)!)
@@ -128,7 +128,7 @@ describe('Demonstrate the Sapling API', () => {
     transaction.spend(decryptedNote, witness)
     transaction.output(newNote)
 
-    const postedTransaction = new TransactionPosted(transaction.post(key.publicAddress, 5n))
+    const postedTransaction = new TransactionPosted(transaction.post(key.spendingKey, key.publicAddress, 5n))
 
     expect(postedTransaction.expiration()).toEqual(10)
     expect(postedTransaction.fee()).toEqual(5n)

ironfish-rust-nodejs/tests/transaction.test.slow.ts

@@ -10,19 +10,19 @@ describe('Transaction', () => {
       const key = generateKey()
       const asset = new Asset(key.publicAddress, 'testcoin', '')
       // Version 1 transactions cannot have an ownership transfer
-      const proposedTx = new Transaction(key.spendingKey, 1)
+      const proposedTx = new Transaction(1)
       proposedTx.mint(asset, 5n, key.publicAddress)
 
-      expect(() => { proposedTx.post(null, 0n)}).toThrow('InvalidTransactionVersion')
+      expect(() => { proposedTx.post(key.spendingKey, null, 0n)}).toThrow('InvalidTransactionVersion')
     })
 
     it('can post a valid transaction', () => {
       const key = generateKey()
       const asset = new Asset(key.publicAddress, 'testcoin', '')
-      const proposedTx = new Transaction(key.spendingKey, 1)
+      const proposedTx = new Transaction(1)
       proposedTx.mint(asset, 5n)
 
-      expect(() => { proposedTx.post(null, 0n)}).not.toThrow()
+      expect(() => { proposedTx.post(key.spendingKey, null, 0n)}).not.toThrow()
 
     })
   })