Attempting to add support for InvoiceID, failing to parse transaction though unclear why

[hyperbolist]

I am ignorant of C in general, and embedded C in particular. But I've attempted to add support for signing Ripple transactions containing InvoiceID anyway. As far as I can tell this should work, but instead it fails to parse for some reason that I have not yet been able to discover.

I'm sure my error is obvious and clear to everyone else, but right now I lack the eyes to see it.

Here's a git applyable patch for xrpParse.c that accepts a Ripple transaction with an InvoiceID as valid input for handleSign:
invoice-id-patch.txt

diff --git a/src/xrpParse.c b/src/xrpParse.c
index 534ab2e..c29143c 100644
--- a/src/xrpParse.c
+++ b/src/xrpParse.c
@@ -22,6 +22,7 @@
 #define STI_AMOUNT 0x06
 #define STI_VL 0x07
 #define STI_ACCOUNT 0x08
+#define STI_HASH256 0x05

 #define XRP_UINT16_TRANSACTION_TYPE 0x02
 #define XRP_UINT32_FLAGS 0x02
@@ -34,6 +35,7 @@
 #define XRP_VL_SIGNING_PUB_KEY 0x03
 #define XRP_ACCOUNT_ACCOUNT 0x01
 #define XRP_ACCOUNT_DESTINATION 0x03
+#define XRP_HASH256_INVOICE_ID 0x11

 void parse_xrp_amount(uint64_t *value, uint8_t *data) {
     *value = ((uint64_t)data[7]) | ((uint64_t)data[6] << 8) |
@@ -214,6 +216,44 @@ error:
     return result;
 }

+parserStatus_e processHash256(uint8_t *data, uint32_t length,
+                              txContent_t *context, uint32_t *offsetParam) {
+    parserStatus_e result = USTREAM_FAULT;
+    uint32_t offset = *offsetParam;
+    uint8_t fieldId = data[offset] & 0x0f;
+    uint8_t dataLength = 32;
+    if ((offset + 1 + dataLength) > length) {
+        result = USTREAM_PROCESSING;
+        goto error;
+    }
+    offset++;
+    switch (fieldId) {
+    case 0: {
+        uint8_t fieldId2 = data[offset];
+        if ((offset + 1 + dataLength) > length) {
+            result = USTREAM_PROCESSING;
+            goto error;
+        }
+        offset++;
+        switch (fieldId2) {
+        case XRP_HASH256_INVOICE_ID:
+            // TODO : display invoiceId on device for confirmation
+            break;
+        default:
+            goto error;
+        }
+        break;
+    }
+
+    default:
+        goto error;
+    }
+    *offsetParam = offset + dataLength;
+    result = USTREAM_FINISHED;
+error:
+    return result;
+}
+
 parserStatus_e parseTxInternal(uint8_t *data, uint32_t length,
                                txContent_t *context) {
     uint32_t offset = 0;
@@ -239,6 +279,9 @@ parserStatus_e parseTxInternal(uint8_t *data, uint32_t length,
         case STI_ACCOUNT:
             result = processAccount(data, length, context, &offset);
             break;
+        case STI_HASH256:
+            result = processHash256(data, length, context, &offset);
+            break;
         default:
             goto error;
         }

As you can see it merely detects the field is present, and moves the offset.

But it still throws a 6a80 because the parse result is not USTREAM_FINISHED by the time parseTx is done, and I can't see why. I've verified that processHash256 merely catches the field and moves the offset, by throwing the next byte in the status message in a kind of primitive console.log-style debugging session. I don't see why there would be additional side effects.

If I let handleSign ignore the invalid parse status, the device will happily ask the user for confirmation, and the fields all appear to be correct on the device's display including fields that come after the InvoiceID in the tx blob, but that produces a different kind of failure by sending more IO that I also don't understand.

I've been testing with @ledgerhq/hw-app-xrp and a nano s.

Here's the debug output of signing a transaction with no InvoiceID.

txJSON:
{ TransactionType: 'Payment',
  Account: 'r...',
  Destination: 'r...',
  Amount: '1',
  Flags: 2147483648,
  LastLedgerSequence: 6242469,
  Fee: '12',
  Sequence: 2,
  SigningPubKey: '03...' }
  
txBLOB:
12000022800000002400000002201B005F40A561400000000000000168400000000000000C732103FBA31B07B997C7C7E72AE378FCA376265B64E5190EFFEA0D2872FB270324205381148C26A748819CBD05D5E3205E51801D9B409C00BC8314000000000000000000000000000000015F953B5B

=>0101050000008ee004004089058000002c8000009080000000000000000000000012000022800000002400000002201b005f40a5614000000000000001684000
=>010105000100000000000c732103fba31b07b997c7c7e72ae378fca376265b64e5190effea0d2872fb270324205381148c26a748819cbd05d5e3205e51801d9b
=>0101050002409c00bc8314000000000000000000000000000000015f953b5b000000000000000000000000000000000000000000000000000000000000000000

<=010105000000483044022053efbfca23608d501036728ae21ce1d959612d32c13684eaddd2696b20899fda02203b375a5506ca09d2570d022b51b67316916662
<=0101050001e9282b22e78216fd8c0a4df52990000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

Here's the debug output of attempting to sign the same transaction with an InvoiceID, which fails to parse, despite my additional code appearing to be valid.

txJSON:
{ TransactionType: 'Payment',
  Account: 'r...',
  Destination: 'r...',
  Amount: '1',
  Flags: 2147483648,
  InvoiceID: '72B3BBCD268B2B838E19B9073239D06811CF2B1CBA14CFF071FAB5434BA5C979',
  LastLedgerSequence: 6242469,
  Fee: '12',
  Sequence: 2,
  SigningPubKey: '03...' }
  
txBLOB:
12000022800000002400000002201B005F40A5501172B3BBCD268B2B838E19B9073239D06811CF2B1CBA14CFF071FAB5434BA5C97961400000000000000168400000000000000C732103FBA31B07B997C7C7E72AE378FCA376265B64E5190EFFEA0D2872FB270324205381148C26A748819CBD05D5E3205E51801D9B409C00BC8314000000000000000000000000000000015F953B5B

=>0101050000009be004004096058000002c8000009080000000000000000000000012000022800000002400000002201b005f40a5501172b3bbcd268b2b838e19
=>0101050001b9073239d06811cf2b1cba14cff071fab5434ba5c97961400000000000000168400000000000000c732103fba31b07b997c7c7e72ae378fca37626
=>01010500025b64e5190effea0d2872fb270324205381148c26a748819cbd05d5e3205e51801d9b409c00bc830000000000000000000000000000000000000000

<=010105000000026a8000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

Here's the debug output of attempting to sign a similar transaction (merely differs by LastLedgerSequence) with an InvoiceID, ignoring the parse error, seemingly returning a valid signature, but then continuing with some additional IO that causes the entire apdu exchange to fail.

txJSON:
{ TransactionType: 'Payment',
  Account: 'r...',
  Destination: 'r...',
  Amount: '1',
  Flags: 2147483648,
  InvoiceID: '72B3BBCD268B2B838E19B9073239D06811CF2B1CBA14CFF071FAB5434BA5C979',
  LastLedgerSequence: 6242393,
  Fee: '12',
  Sequence: 2,
  SigningPubKey: '03...' }

txBLOB:  
12000022800000002400000002201B005F4059501172B3BBCD268B2B838E19B9073239D06811CF2B1CBA14CFF071FAB5434BA5C97961400000000000000168400000000000000C732103FBA31B07B997C7C7E72AE378FCA376265B64E5190EFFEA0D2872FB270324205381148C26A748819CBD05D5E3205E51801D9B409C00BC8314000000000000000000000000000000015F953B5B


=>0101050000009be004004096058000002c8000009080000000000000000000000012000022800000002400000002201b005f4059501172b3bbcd268b2b838e19
=>0101050001b9073239d06811cf2b1cba14cff071fab5434ba5c97961400000000000000168400000000000000c732103fba31b07b997c7c7e72ae378fca37626
=>01010500025b64e5190effea0d2872fb270324205381148c26a748819cbd05d5e3205e51801d9b409c00bc830000000000000000000000000000000000000000

Confirmation display is accurate on the device.

<=01010500000048304402205c3fcda39f693525160413f763be485c1cc7a0fc7deecbf3dd11a183acc57f2c0220767f7b863c8352a4f6dfcda161999baa5b06d2
<=01010500017e9534560ea5ea5dcdb63a42ff90000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

Seems like a successful signTransaction response.

=>0101050000001ae00480401514000000000000000000000000000000015f953b5b00000000000000000000000000000000000000000000000000000000000000

Where is this additional send coming from?

<=010105000000026b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

Here's a 6b00, invalid input response, presumably specifically referring to the send immediately above.

I appreciate any assistance.

[btchip]

Fixed in v2 app