Change Note memos from string to 32 byte buffer (#4743)

* changes NativeNote napi to take memo as buffer (#4740) * changes NativeNote napi to take memo as buffer the NativeNote napi constructor takes the memo to be a utf-8 string that is then converted to bytes when constructing a Memo this prevents clients from easily constructing a memo from arbitrary bytes changes constructor to take the memo as a JsBuffer clients treating memos as strings should convert to buffers before constructing Notes * makes memo byte processing uniform with assetid changes From trait implementation to use byte array of length 32 * Start changing memo from string to Buffer * Remove unused memo_length * Push up wallet test changes * Push up blockchain test fix * fixes assorted test uses of string memos * Fixed syntax error * fixes napi handling of copying bytes from short memos * fixes wallet.test.slow.ts * Change buffer length to max check * Add test for sendTransaction * adds tests for createTransaction use hex string in memoHex to specify memo enforce maximum memo length * fixes memos in demo.test.slow * changes test memo for consistency * adds createTransaction test with no memo * makes memoHex optional in createTransaction yup schema * allows only one of memo or memoHex updates createTransaction and sendTransaction, adds test for each * Change createMinersFee task to take buffer memo * Change buffer to 0 length * Change ternary to if statements * Change ternary in sendTransaction to ifs * Fix broken tests --------- Co-authored-by: Hugh Cunningham <[email protected]> Co-authored-by: Hugh Cunningham <[email protected]>
iron-fish · Feb 21, 2024 · 895a536 · 895a536
1 parent 30ec913
commit 895a536
Show file tree

Hide file tree

Showing 33 changed files with 465 additions and 87 deletions.
diff --git a/ironfish-rust-nodejs/index.d.ts b/ironfish-rust-nodejs/index.d.ts
@@ -165,7 +165,7 @@ export class NoteEncrypted {
 }
 export type NativeNote = Note
 export class Note {
-  constructor(owner: string, value: bigint, memo: string, assetId: Buffer, sender: string)
+  constructor(owner: string, value: bigint, memo: Buffer, assetId: Buffer, sender: string)
   static deserialize(jsBytes: Buffer): NativeNote
   serialize(): Buffer
   /**

diff --git a/ironfish-rust-nodejs/src/structs/note.rs b/ironfish-rust-nodejs/src/structs/note.rs
@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
+use std::cmp;
+
 use ironfish::{
     assets::asset::ID_LENGTH as ASSET_ID_LENGTH,
     note::{AMOUNT_VALUE_SIZE, MEMO_SIZE, SCALAR_SIZE},
@@ -54,22 +56,34 @@ impl NativeNote {
     pub fn new(
         owner: String,
         value: BigInt,
-        memo: String,
+        memo: JsBuffer,
         asset_id: JsBuffer,
         sender: String,
     ) -> Result<Self> {
         let value_u64 = value.get_u64().1;
         let owner_address = ironfish::PublicAddress::from_hex(&owner).map_err(to_napi_err)?;
         let sender_address = ironfish::PublicAddress::from_hex(&sender).map_err(to_napi_err)?;
 
+        let memo_buffer = memo.into_value()?;
+        let memo_vec = memo_buffer.as_ref();
+        let num_to_copy = cmp::min(memo_vec.len(), MEMO_SIZE);
+        let mut memo_bytes = [0; MEMO_SIZE];
+        memo_bytes[..num_to_copy].copy_from_slice(&memo_vec[..num_to_copy]);
+
         let buffer = asset_id.into_value()?;
         let asset_id_vec = buffer.as_ref();
         let mut asset_id_bytes = [0; ASSET_ID_LENGTH];
         asset_id_bytes.clone_from_slice(&asset_id_vec[0..ASSET_ID_LENGTH]);
         let asset_id = asset_id_bytes.try_into().map_err(to_napi_err)?;
 
         Ok(NativeNote {
-            note: Note::new(owner_address, value_u64, memo, asset_id, sender_address),
+            note: Note::new(
+                owner_address,
+                value_u64,
+                memo_bytes,
+                asset_id,
+                sender_address,
+            ),
         })
     }
 

diff --git a/ironfish-rust-nodejs/tests/demo.test.slow.ts b/ironfish-rust-nodejs/tests/demo.test.slow.ts
@@ -59,7 +59,7 @@ describe('Demonstrate the Sapling API', () => {
     const key = generateKey()
 
     const transaction = new Transaction(LATEST_TRANSACTION_VERSION)
-    const note = new Note(key.publicAddress, 20n, 'test', Asset.nativeId(), key.publicAddress)
+    const note = new Note(key.publicAddress, 20n, Buffer.from('test'), Asset.nativeId(), key.publicAddress)
     transaction.output(note)
 
     const serializedPostedTransaction = transaction.post_miners_fee(key.spendingKey)
@@ -96,7 +96,7 @@ describe('Demonstrate the Sapling API', () => {
     const recipientKey = generateKey()
 
     const minersFeeTransaction = new Transaction(LATEST_TRANSACTION_VERSION)
-    const minersFeeNote = new Note(key.publicAddress, 20n, 'miner', Asset.nativeId(), key.publicAddress)
+    const minersFeeNote = new Note(key.publicAddress, 20n, Buffer.from('miner'), Asset.nativeId(), key.publicAddress)
     minersFeeTransaction.output(minersFeeNote)
 
     const postedMinersFeeTransaction = new TransactionPosted(minersFeeTransaction.post_miners_fee(key.spendingKey))
@@ -105,7 +105,7 @@ describe('Demonstrate the Sapling API', () => {
     transaction.setExpiration(10)
     const encryptedNote = new NoteEncrypted(postedMinersFeeTransaction.getNote(0))
     const decryptedNote = Note.deserialize(encryptedNote.decryptNoteForOwner(key.incomingViewKey)!)
-    const newNote = new Note(recipientKey.publicAddress, 15n, 'receive', Asset.nativeId(), minersFeeNote.owner())
+    const newNote = new Note(recipientKey.publicAddress, 15n, Buffer.from('receive'), Asset.nativeId(), minersFeeNote.owner())
 
     let currentHash = encryptedNote.hash()
     let authPath = Array.from({ length: 32 }, (_, depth) => {

diff --git a/ironfish-rust/src/note.rs b/ironfish-rust/src/note.rs
@@ -57,6 +57,12 @@ impl From<String> for Memo {
     }
 }
 
+impl From<[u8; MEMO_SIZE]> for Memo {
+    fn from(value: [u8; MEMO_SIZE]) -> Self {
+        Memo(value)
+    }
+}
+
 impl fmt::Display for Memo {
     /// This can be lossy because it assumes that the
     /// memo is in valid UTF-8 format.

diff --git a/ironfish/src/blockchain/blockchain.test.ts b/ironfish/src/blockchain/blockchain.test.ts
@@ -1718,7 +1718,7 @@ describe('Blockchain', () => {
                 new NativeNote(
                   account.publicAddress,
                   3n,
-                  '',
+                  Buffer.alloc(32),
                   assetId,
                   account.publicAddress,
                 ).serialize(),

diff --git a/ironfish/src/blockchain/blockchain.ts b/ironfish/src/blockchain/blockchain.ts
@@ -1538,7 +1538,12 @@ export class Blockchain {
     const amount = totalTransactionFees + BigInt(reward)
 
     const transactionVersion = this.consensus.getActiveTransactionVersion(blockSequence)
-    return this.workerPool.createMinersFee(minerSpendKey, amount, '', transactionVersion)
+    return this.workerPool.createMinersFee(
+      minerSpendKey,
+      amount,
+      Buffer.alloc(0),
+      transactionVersion,
+    )
   }
 
   newBlockHeaderFromRaw(

diff --git a/ironfish/src/consensus/verifier.test.ts b/ironfish/src/consensus/verifier.test.ts
@@ -245,14 +245,14 @@ describe('Verifier', () => {
           const minerNote1 = new NativeNote(
             owner,
             BigInt(reward / 2),
-            '',
+            Buffer.from(''),
             Asset.nativeId(),
             owner,
           )
           const minerNote2 = new NativeNote(
             owner,
             BigInt(reward / 2),
-            '',
+            Buffer.from(''),
             Asset.nativeId(),
             owner,
           )

diff --git a/ironfish/src/genesis/addGenesisTransaction.ts b/ironfish/src/genesis/addGenesisTransaction.ts
@@ -101,7 +101,7 @@ export async function addGenesisTransaction(
     const note = new NativeNote(
       alloc.publicAddress,
       BigInt(alloc.amountInOre),
-      alloc.memo,
+      Buffer.from(alloc.memo, 'hex'),
       Asset.nativeId(),
       account.publicAddress,
     )

diff --git a/ironfish/src/genesis/makeGenesisBlock.ts b/ironfish/src/genesis/makeGenesisBlock.ts
@@ -56,7 +56,7 @@ export async function makeGenesisBlock(
   const genesisNote = new NativeNote(
     genesisKey.publicAddress,
     allocationSum,
-    '',
+    Buffer.alloc(0),
     Asset.nativeId(),
     genesisKey.publicAddress,
   )
@@ -72,7 +72,7 @@ export async function makeGenesisBlock(
   const note = new NativeNote(
     minersFeeKey.publicAddress,
     BigInt(0),
-    '',
+    Buffer.alloc(0),
     Asset.nativeId(),
     minersFeeKey.publicAddress,
   )
@@ -141,7 +141,7 @@ export async function makeGenesisBlock(
     const note = new NativeNote(
       alloc.publicAddress,
       BigInt(alloc.amountInOre),
-      alloc.memo,
+      Buffer.from(alloc.memo, 'hex'),
       Asset.nativeId(),
       genesisNote.owner(),
     )

diff --git a/ironfish/src/primitives/rawTransaction.test.slow.ts b/ironfish/src/primitives/rawTransaction.test.slow.ts
@@ -46,7 +46,7 @@ function createTestRawTransaction(
     new NativeNote(
       account.publicAddress,
       123456789n,
-      'some memo',
+      Buffer.from('some memo'),
       Asset.nativeId(),
       generateKey().publicAddress,
     ).serialize(),
@@ -102,7 +102,7 @@ function createTestRawTransaction(
       new NativeNote(
         account.publicAddress,
         123456789n,
-        'some memo',
+        Buffer.from('some memo'),
         TEST_ASSET_ID_1,
         generateKey().publicAddress,
       ).serialize(),
@@ -112,7 +112,7 @@ function createTestRawTransaction(
       new NativeNote(
         account.publicAddress,
         123456789n,
-        'some memo',
+        Buffer.from('some memo'),
         TEST_ASSET_ID_2,
         generateKey().publicAddress,
       ).serialize(),
@@ -130,7 +130,7 @@ function createTestRawTransaction(
       new NativeNote(
         generateKey().publicAddress,
         123456789n - raw.fee,
-        'some memo',
+        Buffer.from('some memo'),
         Asset.nativeId(),
         account.publicAddress,
       ).serialize(),

diff --git a/ironfish/src/primitives/rawTransaction.test.ts b/ironfish/src/primitives/rawTransaction.test.ts
@@ -212,7 +212,7 @@ describe('RawTransactionSerde', () => {
       new NativeNote(
         generateKey().publicAddress,
         5n,
-        'memo',
+        Buffer.from('memo'),
         asset.id(),
         account.publicAddress,
       ).serialize(),
@@ -290,7 +290,7 @@ describe('RawTransactionSerde', () => {
       new NativeNote(
         generateKey().publicAddress,
         5n,
-        'memo',
+        Buffer.from('memo'),
         asset.id(),
         accountA.publicAddress,
       ).serialize(),

diff --git a/ironfish/src/primitives/transaction.test.perf.ts b/ironfish/src/primitives/transaction.test.perf.ts
@@ -84,13 +84,13 @@ describe('Transaction', () => {
     const spendAmount = BigInt(numSpends) * CurrencyUtils.decodeIron(20)
     const outputAmount = BigIntUtils.divide(spendAmount, BigInt(numOutputs))
 
-    const outputs: { publicAddress: string; amount: bigint; memo: string; assetId: Buffer }[] =
+    const outputs: { publicAddress: string; amount: bigint; memo: Buffer; assetId: Buffer }[] =
       []
     for (let i = 0; i < numOutputs; i++) {
       outputs.push({
         publicAddress: account.publicAddress,
         amount: BigInt(outputAmount),
-        memo: '',
+        memo: Buffer.from(''),
         assetId: Asset.nativeId(),
       })
     }

diff --git a/ironfish/src/primitives/transaction.test.slow.ts b/ironfish/src/primitives/transaction.test.slow.ts
@@ -61,7 +61,7 @@ describe('Transaction', () => {
         {
           publicAddress: accountB.publicAddress,
           amount: BigInt(1),
-          memo: '',
+          memo: Buffer.from(''),
           assetId: Asset.nativeId(),
         },
       ],
@@ -86,7 +86,7 @@ describe('Transaction', () => {
     const expiration = 10
     const amount = 10n
     const burnAmount = 1n
-    const memo = 'Hello World'
+    const memo = Buffer.from('Hello World')
     const assetName = 'Testcoin'
     const metadata = 'testcoin metadata'
     const asset = new Asset(account.publicAddress, assetName, metadata)

diff --git a/ironfish/src/primitives/transactionVerify.test.perf.ts b/ironfish/src/primitives/transactionVerify.test.perf.ts
@@ -123,13 +123,13 @@ describe('Verify Transaction', () => {
     const spendAmount = BigInt(numSpends) * CurrencyUtils.decodeIron(20)
     const outputAmount = BigIntUtils.divide(spendAmount, BigInt(numOutputs))
 
-    const outputs: { publicAddress: string; amount: bigint; memo: string; assetId: Buffer }[] =
+    const outputs: { publicAddress: string; amount: bigint; memo: Buffer; assetId: Buffer }[] =
       []
     for (let i = 0; i < numOutputs; i++) {
       outputs.push({
         publicAddress: account.publicAddress,
         amount: BigInt(outputAmount),
-        memo: '',
+        memo: Buffer.from(''),
         assetId: Asset.nativeId(),
       })
     }

diff --git a/ironfish/src/rpc/routes/wallet/__fixtures__/createTransaction.test.ts.fixture b/ironfish/src/rpc/routes/wallet/__fixtures__/createTransaction.test.ts.fixture
@@ -838,5 +838,97 @@
         }
       ]
     }
+  ],
+  "Route wallet/createTransaction should create transaction using memoHex": [
+    {
+      "version": 4,
+      "id": "65f292fe-447b-4fdc-9f5d-1d87c679d878",
+      "name": "existingAccount",
+      "spendingKey": "d1f0fa57871b472b27632df610a969d2b17482e140c5582368d56a34671b2f19",
+      "viewKey": "9b053fb9039ecbbed113b60f4125749c1b4e4954e2dcce7f754f8d508fc4737048f54f4b94bad2afc837bf36602a5650b605f2a8496dbf4c91db1f86c87fd770",
+      "incomingViewKey": "381d6f627054582fbe93b21e8536a5a97fe0c46f18e8afde66b0af3d0397cd06",
+      "outgoingViewKey": "0247cefeb6b8f4d581e2432acdc44e21fe9dc77fd6249bc64568257853b5e729",
+      "publicAddress": "a1edf124480c1c5f256a6f7e19424bcb3a8d680977c966f990fde74ec01365e9",
+      "createdAt": {
+        "hash": {
+          "type": "Buffer",
+          "data": "base64:R5HXrp+X3xAO8VWOhHctagm0N2I4goP3XG8goyqIqoY="
+        },
+        "sequence": 1
+      },
+      "proofAuthorizingKey": "8779d66619335e2335e87b8222776695925cc6589f2ff74a7dc3e14ee62f0703"
+    },
+    {
+      "header": {
+        "sequence": 2,
+        "previousBlockHash": "4791D7AE9F97DF100EF1558E84772D6A09B43762388283F75C6F20A32A88AA86",
+        "noteCommitment": {
+          "type": "Buffer",
+          "data": "base64:EL+5Xwlj0WiPwsQ49awyIAJ39N6n436tcyPPRR5R8TY="
+        },
+        "transactionCommitment": {
+          "type": "Buffer",
+          "data": "base64:2gMem3TOMZHj4Qrlal7qhDd3i0xw5IKQqqzkVa88CtA="
+        },
+        "target": "9282972777491357380673661573939192202192629606981189395159182914949423",
+        "randomness": "0",
+        "timestamp": 1708044968503,
+        "graffiti": "0000000000000000000000000000000000000000000000000000000000000000",
+        "noteSize": 4,
+        "work": "0"
+      },
+      "transactions": [
+        {
+          "type": "Buffer",
+          "data": "base64:AgAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAGzKiP////8AAAAAHl6TyxC6GpTvkB5Wf5vzk9mqaOEhEsDTA3WMBcM65XKw+cgRFJUfhpo4SFSQ2kRwWgrJwnMhf3hvkzjOkdUoD3bvtEsL/+x+e9rMuQYQiueVJhbkGZ326NYmn0SBHL+0Td0ZfAkdvh+is9mnpUJGeORFjKDRMwYcp69CE+LroI4Nz/7Fh1Jww4BO875FKdsIyWlvWlA0Kha8PrC4kbh/r0nUWC5QOdCgjyGFWnWbgMmQNYJnzaa5X+IsB/yTOScJLbNyq4fK9hVWKlY0J2NE1glupWCQeeD+r1MNJXVV/4SBhulGpuK857tIe3B0oYISfiwyLGUA03LBhx0ILgwlLZa65dPEZyUl5VqBV+6zq9wV+0kZ9xAhiXGm47+HLHJNNrU2dNbq7PKGkq8+zwg4l9x+WNJux2IvC/5yCpfmTeSd/f5+Ldd0YqpRu+JSEY3NQT5rxGAVakmXwZebXHMJNxolt0SK37qnhE9cD6NoqRjzo/4UhK36rR98uIslMnfTcAMvHwFgmnNQluCeWQs9u7CYFNsxP9rxXO+tdEAzSU7jcb5C1yVkXW54Bl0/9/mSUjTVcKJqthADr9Uv2gNKDub/HdcqGR7ojCTnJpZTnDFLnPwwvL07KUlyb24gRmlzaCBub3RlIGVuY3J5cHRpb24gbWluZXIga2V5MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw4DPc6qEcpuUdHhUOah+t/QdZrujnul5cMg+Sc8HM9nEsIVuUI7enLPsLNuQAWCPnEaD0QdE2NMV2fLduZ9h3AA=="
+        }
+      ]
+    }
+  ],
+  "Route wallet/createTransaction should create transaction with no memo": [
+    {
+      "version": 4,
+      "id": "b3e4c164-c479-4dd9-a8a2-36f754d430d8",
+      "name": "existingAccount",
+      "spendingKey": "29aba0d30212cdcff8b50163e0e647b7615e55299e1886f9dcc79c27f1f6edca",
+      "viewKey": "23b50ea197210babcd7df30c4864ccf45f7851d7cd99cf8b81ae208d2f865909c3255fbd9de37a09fe16355e978d033c49bd9dcfdc4445eb78cfc692c623d91c",
+      "incomingViewKey": "03568087c837bcf481fcf404b6693713df3a26fbb243071b79affabac29ade03",
+      "outgoingViewKey": "caace199576008230996da764a7bce6b1a663665015bb959d1c1c71518649e93",
+      "publicAddress": "7741877c7c4771b5304d04a7bd6c2662c97d295962981eeeeb85b6fa113fba86",
+      "createdAt": {
+        "hash": {
+          "type": "Buffer",
+          "data": "base64:R5HXrp+X3xAO8VWOhHctagm0N2I4goP3XG8goyqIqoY="
+        },
+        "sequence": 1
+      },
+      "proofAuthorizingKey": "1bbcbba0e6170165c90753badcfb85554c77fd740847fcb4ab065347fadc5b06"
+    },
+    {
+      "header": {
+        "sequence": 2,
+        "previousBlockHash": "4791D7AE9F97DF100EF1558E84772D6A09B43762388283F75C6F20A32A88AA86",
+        "noteCommitment": {
+          "type": "Buffer",
+          "data": "base64:tMR7WCLDXwulsqXQYgRWmqN9wR28LdWUxVL0nNDIVWE="
+        },
+        "transactionCommitment": {
+          "type": "Buffer",
+          "data": "base64:iJ2RTsbkXO0yziEuDD6jjKg7h6NJFXQVTCvHb4kxI1Q="
+        },
+        "target": "9282972777491357380673661573939192202192629606981189395159182914949423",
+        "randomness": "0",
+        "timestamp": 1708109450669,
+        "graffiti": "0000000000000000000000000000000000000000000000000000000000000000",
+        "noteSize": 4,
+        "work": "0"
+      },
+      "transactions": [
+        {
+          "type": "Buffer",
+          "data": "base64:AgAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAGzKiP////8AAAAAZ2OGDWLTDMoZGTWUg+9zG/MbUjmVXXJsIEYwrr4OBcCsQlx2VlkfoLnfCxjpbs+NeyNZWTL5jn2a55ri4edKHKzf6Mom1CpKiw1/SZKZ25GsM5PAfFzJ+5QKDjhwBgl4FZBLctLGm5QKrpeRnRW6iblNk2TMKS8NlyB46HyE7sMOnQKavjAdsJiV6gHU11RB2hRlMX8KHAPHZXEkiUZqGrZsYEu7Yz0NypbNQCxgPhCwoJjHp91uyBRgWnqpPigv3erWxYJERQzLC5aGFWc7erj2qwyuv9Pxw8o/QNR8mt9Ldp9U5XQUtwgTCro24TqLZc6ZerUtfTODAnbwnzvWsLSd8oHiz+8WkDWkt2aCY52yJvReDZSjzjOpJTa5YwMw7vmZ3isOxVx9FYjRbGGsO+U0LPx8JImeHx6dS+zbJYAKtlQ/BPi6ivFv+/C3FYD8bZbJA5Oa5wh1GblzJjxqSXQ4hev6gOHPzrke0pl3UW01Ctvbf47iYsxLtahevdWcGm40bkdF2UMgqFR1TsKGI2SU7qSmG5OhgM2K9SKOr/PYB6BlGEiGL9nPOyYu/VpAz6DCcTNP40MRSY5gddv1ivreDOBKdv3wikWJ8gNQzMKOKlglKGOICUlyb24gRmlzaCBub3RlIGVuY3J5cHRpb24gbWluZXIga2V5MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwSId5xa1d0KuLJG6wdXu/lcE6BT9N3cXx3xiT6kI/RyJZxnyjNLtTt34ta8CjrvVsU8n6ysi9LTDp3cI7eaiwAA=="
+        }
+      ]
+    }
   ]
 }