Version v0.6.9: SDK upgrade, Ledger Stax UI, Improve Blind Signing behavior

[vldmkr]

Description

Features:

• [Nano S/SP/X] Always inform about the blind signing, even if the option is activated in the settings.
• [Nano S/SP/X] Invite users to enable the blind signing setting during the signature request.
• Upgrade to a newer version of the SDK
• Add MAX_TRANSACTION_PACKETS to Makefile for TARGET_STAX
• Implement the UI for Ledger Stax
  • Separate the common UI code for BAGL and NBGL
  • Put general functions into utils
  • Refactor BAGL UI (display, menu, settings)
  • Implement NBGL UI (display, menu, settings)
  • Add Aptos icon for Ledger Stax
• Expand the displayed info
  • Parse 0x1::aptos_account::transfer_coins function
  • Now show "Transaction Type" for all entry_function calls
  • Add support for multisig payload variant
• Add new tests.
• Update workflows for misspellings checks, coding style checks, documentation generation, clang static analyzer, Python client checks, CodeQL checks, build and functional tests.
• Version bump to v0.6.9.

Fixes:

• [v0.6.2] Fix typo on Blind Signing warning screen
• [v0.6.2] Fix the tests, remove the obsolete ones after an upgrade
• [v0.6.3] Use "no_throw" crypto functions instead of deprecated ones
• [v0.6.6] Rewrite existing tests using the Ragger framework.

Changes include

• Bugfix (non-breaking change that solves an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (change that is not backwards-compatible and/or changes current functionality)
• Tests
• Documentation
• Other (for changes that might not fit in any category)

[codecov-commenter]

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (846999c) 63.46% compared to head (57e4c93) 42.58%.

Files	Patch %	Lines
src/transaction/deserialize.c	18.18%	9 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           develop      #13       +/-   ##
============================================
- Coverage    63.46%   42.58%   -20.89%     
============================================
  Files           13        5        -8     
  Lines          802      418      -384     
============================================
- Hits           509      178      -331     
+ Misses         293      240       -53     

Flag	Coverage Δ
unittests	42.58% <18.18%> (-20.89%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[junkil-park]

@vldmkr , These look like great improvements to the app. I am particularly interested in the multisig payload support. Can I know what the release timeline for Version v0.6.9 is?

[hardsetting]

Thanks for including my changes @vldmkr!
Is there anything I can help you with in order to get this merged, maybe a PR review?

[hardsetting]

I also have a small question, not necessarily related to this PR (although if it's quick and we can get it in, that'd be amazing).

What is the reasoning for requiring a message to be UTF-8 encoded?

app-aptos/src/transaction/deserialize.c

Lines 127 to 131 in 846999c

	if (transaction_utils_check_encoding(buf->ptr, buf->size)) {
	buf->offset = 0;
	tx->tx_variant = TX_MESSAGE;
	return PARSING_OK;
	}

We've gotten requests from people that were looking into doing key rotation, or other operations that involve signing a structured message (not UTF-8).

Removing this limitation would effectively unblock them.

[vldmkr]

@vldmkr , These look like great improvements to the app. I am particularly interested in the multisig payload support. Can I know what the release timeline for Version v0.6.9 is?

@junkil-park Thank you! At the moment the app source code is undergoing a security audit. I believe that once this iterative process is over, the Ledger team will accept the PR shortly. We are all eagerly awaiting this!

[vldmkr]

Thanks for including my changes @vldmkr! Is there anything I can help you with in order to get this merged, maybe a PR review?

@hardsetting I truly appreciate your willingness to help! Today we received recommendations from the auditors to make some fixes. It does not look time-consuming, so it will be ready early next week, and I hope we will be close to the final.

What is the reasoning for requiring a message to be UTF-8 encoded?

There were a couple of reasons for this in the initial design:

1. Users must be able to visually confirm what they are about to sign. This means that the wallet app initiating the request and the Ledger app must display the information identically. If we have a UTF-8 message, we don't need to convert it additionally and we can display the information directly from the transaction buffer. If this is not possible, it should be considered as "blind signing".
   

   app-aptos/src/ui/bagl_display.c

   Lines 196 to 202 in 6f78868

   	// Step with title/text for message
   	UX_STEP_NOCB(ux_display_msg_step,
   	bnnn_paging,
   	{
   	.title = "Message",
   	.text = (const char *) G_context.tx_info.raw_tx,
   	});

2. If we want to display a non-UTF-8 message, we first need to encode it in (for example) HEX and put it into a buffer, which is limited to 120 bytes. If it is HEX encoded, we can only display the first 60 bytes of useful information. So in most cases we still end up in a "blind signing" case.
   

   app-aptos/src/ui/display.h

   Line 9 in 6f78868

   extern char g_struct[120];

   Just a reminder that since Aptos uses EdDSA without pre-hashing, we first need to get all the data and pass it to the system function for signing in its raw form. Therefore, we cannot build the hash in the stream receiving the update packets and are limited by the amount of RAM on the device.
   

   app-aptos/src/crypto.c

   Lines 88 to 93 in 6f78868

   	error = cx_eddsa_sign_no_throw(&private_key,
   	CX_SHA512,
   	G_context.tx_info.raw_tx,
   	G_context.tx_info.raw_tx_len,
   	G_context.tx_info.signature,
   	sizeof(G_context.tx_info.signature));

   
   

   app-aptos/Makefile

   Lines 28 to 40 in 6f78868

   	ifeq ($(TARGET_NAME),TARGET_NANOS)
   	DEFINES += MAX_TRANSACTION_PACKETS=6
   	endif
   	ifeq ($(TARGET_NAME),TARGET_NANOS2)
   	DEFINES += MAX_TRANSACTION_PACKETS=106
   	endif
   	ifeq ($(TARGET_NAME),TARGET_NANOX)
   	DEFINES += MAX_TRANSACTION_PACKETS=90
   	endif
   	ifeq ($(TARGET_NAME),TARGET_STAX)
   	# still need to find the right value
   	DEFINES += MAX_TRANSACTION_PACKETS=70
   	endif

   
   This is why we aim to make helper buffers as small as possible, so as not to reduce the size of the transaction that the Ledger is able to accept.

Since we will make fixes according to the audit, I also suggest to introduce a new type TX_RAW_MESSAGE and display it as a HEX string, in case the message will be more than 60 bytes and will not fit in the helper buffer, we will display the first 60 bytes with a request to activate blind signing. This allows us to remove the limitation of accepting the message only in UTF-8 format.

[hardsetting]

Thank you @vldmkr for the amazing run down.
I follow your input and implemented the change here.

Changes and tests are in separate commits, hopefully that helps the review.

[vldmkr]

@hardsetting This is an awesome contribution! Thank you for your efforts. Everything looks great on my end, just added some fixes to have a green CI. I finished everything according to the audit and this will be a great addition to this release.

[tdejoigny-ledger]

Jira ticket : B2CA-1555