fix: don't apply eip155 to v anymore

[nicolasbrugneaux]

Checklist

• App update process has been followed
• Target branch is develop
• Application version has been bumped

After investigation with the ledger-live team we came to the conclusion that the newest (1.3.1) version of this app returned an EIP155 formatted v that the ledgerjs sdk (@ledgerhq/hw-app-eth) doesn't expect for typed transaction (see https://github.com/LedgerHQ/ledger-live/blob/develop/libs/ledgerjs/packages/hw-app-eth/src/utils.ts#L107), it expects v to represent to yParity as either a 0 or a 1. This PR aims to fix this issue.

However, I'm not a C developer and there's a line I commented out that I believe is not used anymore but would like to clarity on what it did and if my assumptions are indeed correct.

[gastonponti]

I need to read more about the app that basically uses this one. https://github.com/LedgerHQ/blue-secure-sdk
I'm not completely sure if this is the required fix, to be honest.
Because the legacyTxs will still need the v with chain id. Except that you are switching to only use Dynamic or 64.

On the other hand, I would also keep the +2. The recoverId, or ´v´ could be [0, 1, 2, 3] actually.

   // this part is checking the parity of Y at a bit level against "info" returned by the signer
    if (info & CX_ECCINFO_PARITY_ODD) {
      G_io_apdu_buffer[0]++;
    }
    // and here they are checking if the magnitud of x is grater than the curve order
    if (info & CX_ECCINFO_xGTn) {
      G_io_apdu_buffer[0] += 2;
    }

So actually, v could be:
0: y-parity 0, magnitude of x lower than the curve order
1: y-parity 1, magnitude of x lower than the curve order
2: y-parity 0, magnitude of x greater than the curve order
3: y-parity 1, magnitude of x greater than the curve order

I don't actually know how probable is to have 2 and 3, or if maybe the signers in general retry this until having something without 2 or 3, but it is an actual possibility and this code is taking this into count, so I would let that as it is right now.

[GuilaneDen]

After reviewing the implementation and considering the points raised, here are some insights and recommendations:

1. EIP-1559/CIP-64 Compatibility:

• The latest version of the LedgerJS SDK @ledgerhq/hw-app-eth) adheres to EIP-1559 and CIP-64, which expect v to represent only the yParity (0 for even and 1 for odd parity). This is in alignment with the standards for typed transactions, and as you mentioned, the Embedded App does not currently accommodate this expectation for typed transactions.

• To ensure compatibility, the app could reintroduce conditional logic to distinguish between typed transactions (EIP-1559/CIP-64) and legacy transactions.

2. Legacy Transactions:

• Legacy transactions will no longer be supported by the application. Given the move towards modern standards (e.g., EIP-1559 and CIP-64), there is no need to maintain support for the legacy v format that includes the chain ID. This decision simplifies the implementation and ensures that the app remains forward-compatible with newer protocols.
• As a result:
  • The logic for handling the legacy v calculation, including (v * 2) + 35, can safely be removed.
  • The app will exclusively handle the yParity format, returning v=0 or v=1 for modern transactions.
• This change reflects a deliberate move to streamline the app’s functionality, focusing solely on typed transactions and reducing unnecessary complexity.

3. Magnitude of x and Parity:

• The existing implementation accounts for the additional possibilities of v = [0, 1, 2, 3], based on:
  • y-parity (even or odd).
  • Whether x is greater than the curve order (x>n).
• While it might be rare to encounter cases where x>n, this logic ensures mathematical correctness and robustness, preventing potential edge cases or invalid signatures. Removing the "+2" adjustment could lead to compatibility issues in scenarios where these edge cases are encountered.

4. Recommended Fix:

• Since legacy transactions are no longer supported, the app can use a simplified implementation that always computes v for typed transactions. This eliminates the legacy transaction handling code and ensures that only v0 or v=1 is returned, aligning with the expectations of EIP-1559 and CIP-64. Here’s the updated code:

// Simplified: Typed transactions only (EIP-1559/CIP-64)
G_io_apdu_buffer[0] = 0; // Start with parity even (0)
if (info & CX_ECCINFO_PARITY_ODD) {
    G_io_apdu_buffer[0] = 1; // Adjust for parity odd (1)
}

• If you want an approach that preserves backward compatibility while ensuring adherence to newer standards for typed transactions (which is in your case not necessary). You can use this kind of implementation:

if (is_modern_transaction()) {
    // Typed transactions (EIP-1559/CIP-64): Use parity only
    G_io_apdu_buffer[0] = 0;
    if (info & CX_ECCINFO_PARITY_ODD) {
        G_io_apdu_buffer[0]++;
    }
} else {
    // Legacy transactions: Include chain ID and parity/magnitude
    if (tmpContent.txContent.vLength == 0) { // Legacy API
        G_io_apdu_buffer[0] = 27; 
    } else { // New API
        G_io_apdu_buffer[0] = (v * 2) + 35;
        if (info & CX_ECCINFO_PARITY_ODD) {
            G_io_apdu_buffer[0]++;
        }
        if (info & CX_ECCINFO_xGTn) {
            G_io_apdu_buffer[0] += 2;
        }
    }
}```

[GuilaneDen]

The v constant is not used anymore so you need to delete line 93. "uint32_t v = getV(&tmpContent.txContent);"

[GuilaneDen]

You can delete this commented code

[aaronmgdr]

replace by #24