Ledger.Net

Ledger.Net copied to clipboard

SignChunkedEthereumTransaction Unit Test Hangs

When the APDU data field is greater than 255 bytes, the LC field is truncated to a byte.

RequestBase.cs

               var apdu = new byte[Data.Length + 5];
                apdu[0] = Cla;
                apdu[1] = Ins;
                apdu[2] = Argument1;
                apdu[3] = Argument2;
               apdu[4] = (byte)(Data.Length);  // TRUNCATION
                Array.Copy(Data, 0, apdu, 5, Data.Length);

Looking at the Ethereum wallet app (https://github.com/LedgerHQ/ledger-app-eth/blob/master/src_genericwallet/main.c) , the way to send a command with a data field greater than 255 is by using the P1 parameters to specify that there are more data #define P1_MORE 0x80

Yes. I see. This is a big problem.

@juanfranblanco @ThatSlyGuy , this looks like it is one of the reasons you guys have had so much trouble signing Ethereum transactions. I didn't realise there was still an issue. @edeharo has grasped this issue and there will be a fix at some point soon.

@edeharo , again, thanks for finding this bug and looking in to it. I'm currently quite busy so I won't be able to fix it myself for a week or two. There's no rush to resubmit a pull request on this one, but if you need help, please shout out.

@MelbourneDeveloper, sorry for the detail I was quite busy too. I just created the pull request. Please let me know if you need any clarification.

This issue is almost fixed. One small bug to iron out left. See comments here:

https://github.com/MelbourneDeveloper/Ledger.Net/pull/14

This issue has been fixed in branch https://github.com/MelbourneDeveloper/Ledger.Net/tree/fix-trx-signature-christian and all the unit tests pass. This will be merged in to master and a NuGet will be released in the next couple of days.

@ThatSlyGuy @juanfranblanco

Fixed in NuGet 2.3.0

@edeharo

@edeharo did release 2.3.0 ever fix this issue? I apologise if I closed this without getting it fixed for you.

I'm working on this in branch #13. I think I've sorted out the problem with passing the right parameters. I'm doing this now:

           for (var i = 0; i < apduChunks.Count; i++)
            {
                var apduCommandChunk = apduChunks[i];

                if (apduChunks.Count == 1)
                {
                    //There is only one chunk so use the argument from the request (e.g P1_SIGN)
                    apduCommandChunk[2] = request.Argument1;
                }
                else if (apduChunks.Count > 1)
                {
                    //There are multiple chunks so the assumption is that this is probably a transaction

                    if (i == 0)
                    {
                        //This is the first chunk of the transaction
                        apduCommandChunk[2] = Constants.P1_FIRST;
                    }
                    else if (i == (apduChunks.Count - 1))
                    {
                        //This is the last chunk of the transaction
                        apduCommandChunk[2] = Constants.P1_LAST;
                    }
                    else
                    {
                        //This is one of the middle chunks and there is more coming
                        apduCommandChunk[2] = Constants.P1_MORE;
                    }
                }

                var packetIndex = 0;
                byte[] data = null;
                using (var memoryStream = new MemoryStream(apduCommandChunk))
                {
                    do
                    {
                        data = Helpers.GetRequestDataPacket(memoryStream, packetIndex);
                        packetIndex++;
                        await LedgerHidDevice.WriteAsync(data);
                    } while (memoryStream.Position != memoryStream.Length);
                }

                var responseDataChunk = await ReadAsync();

                responseData.Add(responseDataChunk);

                var returnCode = ResponseBase.GetReturnCode(responseDataChunk);

                if (returnCode != Constants.SuccessStatusCode)
                {
                    return responseData;
                }
            }

But the unit test still fails. Why? It's saying invalid parameter. Which parameter? It's exactly the same code as the other signing example, but the data is different.