From 1dcfdebf7a8f6219f4c085bd180c4d8df1c1c2fe Mon Sep 17 00:00:00 2001 From: matejcik Date: Fri, 6 Aug 2021 14:12:07 +0200 Subject: [PATCH] feat(all): make Stellar timebounds required --- common/protob/messages-stellar.proto | 4 +- core/.changelog.d/1755.changed.1 | 1 + core/.changelog.d/1755.incompatible | 1 + core/src/apps/stellar/sign_tx.py | 22 ++--- core/src/trezor/messages.py | 8 +- .../firmware/.changelog.d/1755.incompatible | 1 + legacy/firmware/stellar.c | 87 +++++++++---------- python/src/trezorlib/messages.py | 12 +-- 8 files changed, 63 insertions(+), 73 deletions(-) create mode 100644 core/.changelog.d/1755.changed.1 create mode 100644 core/.changelog.d/1755.incompatible create mode 100644 legacy/firmware/.changelog.d/1755.incompatible diff --git a/common/protob/messages-stellar.proto b/common/protob/messages-stellar.proto index 7e84f80e7..5be931ec4 100644 --- a/common/protob/messages-stellar.proto +++ b/common/protob/messages-stellar.proto @@ -52,8 +52,8 @@ message StellarSignTx { required string source_account = 4; // source account address required uint32 fee = 5; // Fee (in stroops) for the transaction required uint64 sequence_number = 6; // transaction sequence number - optional uint32 timebounds_start = 8; // unix timestamp (client must truncate this to 32 bytes) - optional uint32 timebounds_end = 9; // unix timestamp (client must truncate this to 32 bytes) + required uint32 timebounds_start = 8; // unix timestamp (client must truncate this to 32 bytes) + required uint32 timebounds_end = 9; // unix timestamp (client must truncate this to 32 bytes) required StellarMemoType memo_type = 10; // type of memo attached to the transaction optional string memo_text = 11; // up to 28 characters (4 bytes are for length) optional uint64 memo_id = 12; // 8-byte uint64 diff --git a/core/.changelog.d/1755.changed.1 b/core/.changelog.d/1755.changed.1 new file mode 100644 index 000000000..47988bad0 --- /dev/null +++ b/core/.changelog.d/1755.changed.1 @@ -0,0 +1 @@ +Most Stellar fields are now required on protobuf level diff --git a/core/.changelog.d/1755.incompatible b/core/.changelog.d/1755.incompatible new file mode 100644 index 000000000..686117f28 --- /dev/null +++ b/core/.changelog.d/1755.incompatible @@ -0,0 +1 @@ +Timebounds must be set for a Stellar transaction diff --git a/core/src/apps/stellar/sign_tx.py b/core/src/apps/stellar/sign_tx.py index 4184f923f..8979391ba 100644 --- a/core/src/apps/stellar/sign_tx.py +++ b/core/src/apps/stellar/sign_tx.py @@ -70,20 +70,14 @@ async def _init(ctx: Context, w: bytearray, pubkey: bytes, msg: StellarSignTx) - ) -async def _timebounds( - ctx: Context, w: bytearray, start: int | None, end: int | None -) -> None: - # timebounds are only present if timebounds_start or timebounds_end is non-zero - if start is not None and end is not None: - # confirm dialog - await layout.require_confirm_timebounds(ctx, start, end) - writers.write_bool(w, True) - - # timebounds are sent as uint32s since that's all we can display, but they must be hashed as 64bit - writers.write_uint64(w, start) - writers.write_uint64(w, end) - else: - writers.write_bool(w, False) +async def _timebounds(ctx: Context, w: bytearray, start: int, end: int) -> None: + # confirm dialog + await layout.require_confirm_timebounds(ctx, start, end) + + # timebounds are sent as uint32s since that's all we can display, but they must be hashed as 64bit + writers.write_bool(w, True) + writers.write_uint64(w, start) + writers.write_uint64(w, end) async def _operations(ctx: Context, w: bytearray, num_operations: int) -> None: diff --git a/core/src/trezor/messages.py b/core/src/trezor/messages.py index b0141717d..d9b9d0637 100644 --- a/core/src/trezor/messages.py +++ b/core/src/trezor/messages.py @@ -4485,8 +4485,8 @@ if TYPE_CHECKING: source_account: "str" fee: "int" sequence_number: "int" - timebounds_start: "int | None" - timebounds_end: "int | None" + timebounds_start: "int" + timebounds_end: "int" memo_type: "StellarMemoType" memo_text: "str | None" memo_id: "int | None" @@ -4500,11 +4500,11 @@ if TYPE_CHECKING: source_account: "str", fee: "int", sequence_number: "int", + timebounds_start: "int", + timebounds_end: "int", memo_type: "StellarMemoType", num_operations: "int", address_n: "list[int] | None" = None, - timebounds_start: "int | None" = None, - timebounds_end: "int | None" = None, memo_text: "str | None" = None, memo_id: "int | None" = None, memo_hash: "bytes | None" = None, diff --git a/legacy/firmware/.changelog.d/1755.incompatible b/legacy/firmware/.changelog.d/1755.incompatible new file mode 100644 index 000000000..686117f28 --- /dev/null +++ b/legacy/firmware/.changelog.d/1755.incompatible @@ -0,0 +1 @@ +Timebounds must be set for a Stellar transaction diff --git a/legacy/firmware/stellar.c b/legacy/firmware/stellar.c index 83e2d14f0..5cfb0e610 100644 --- a/legacy/firmware/stellar.c +++ b/legacy/firmware/stellar.c @@ -101,21 +101,16 @@ bool stellar_signingInit(const StellarSignTx *msg) { // Hash: sequence number stellar_hashupdate_uint64(msg->sequence_number); - // Timebounds are only present if timebounds_start or timebounds_end is - // non-zero - uint8_t has_timebounds = - (msg->timebounds_start > 0 || msg->timebounds_end > 0); // Hash: the "has timebounds?" boolean - stellar_hashupdate_bool(has_timebounds); - if (has_timebounds) { - // Timebounds are sent as uint32s since that's all we can display, but they - // must be hashed as 64-bit values - stellar_hashupdate_uint32(0); - stellar_hashupdate_uint32(msg->timebounds_start); + stellar_hashupdate_bool(true); - stellar_hashupdate_uint32(0); - stellar_hashupdate_uint32(msg->timebounds_end); - } + // Timebounds are sent as uint32s since that's all we can display, but they + // must be hashed as 64-bit values + stellar_hashupdate_uint32(0); + stellar_hashupdate_uint32(msg->timebounds_start); + + stellar_hashupdate_uint32(0); + stellar_hashupdate_uint32(msg->timebounds_end); // Hash: memo stellar_hashupdate_uint32(msg->memo_type); @@ -1297,7 +1292,8 @@ void stellar_format_asset(const StellarAsset *asset, char *str_formatted, memzero(str_asset_issuer_trunc, sizeof(str_asset_issuer_trunc)); // Validate issuer account for non-native assets - if (asset->type != StellarAssetType_NATIVE && !stellar_validateAddress(asset->issuer)) { + if (asset->type != StellarAssetType_NATIVE && + !stellar_validateAddress(asset->issuer)) { stellar_signingAbort(_("Invalid asset issuer")); return; } @@ -1323,7 +1319,8 @@ void stellar_format_asset(const StellarAsset *asset, char *str_formatted, memcpy(str_asset_issuer_trunc, asset->issuer, 5); } // Issuer is read the same way for both types of custom assets - if (asset->type == StellarAssetType_ALPHANUM4 || asset->type == StellarAssetType_ALPHANUM12) { + if (asset->type == StellarAssetType_ALPHANUM4 || + asset->type == StellarAssetType_ALPHANUM12) { strlcat(str_formatted, _(" ("), len); strlcat(str_formatted, str_asset_issuer_trunc, len); strlcat(str_formatted, _(")"), len); @@ -1676,42 +1673,38 @@ void stellar_layoutTransactionSummary(const StellarSignTx *msg) { memzero(str_lines, sizeof(str_lines)); // Timebound: lower - if (msg->timebounds_start || msg->timebounds_end) { - time_t timebound; - char str_timebound[32] = {0}; - const struct tm *tm = NULL; - - timebound = (time_t)msg->timebounds_start; - strlcpy(str_lines[0], _("Valid from:"), sizeof(str_lines[0])); - if (timebound) { - tm = gmtime(&timebound); - strftime(str_timebound, sizeof(str_timebound), "%F %T (UTC)", tm); - strlcpy(str_lines[1], str_timebound, sizeof(str_lines[1])); - } else { - strlcpy(str_lines[1], _("[no restriction]"), sizeof(str_lines[1])); - } + time_t timebound; + char str_timebound[32] = {0}; + const struct tm *tm = NULL; + + timebound = (time_t)msg->timebounds_start; + strlcpy(str_lines[0], _("Valid from:"), sizeof(str_lines[0])); + if (timebound) { + tm = gmtime(&timebound); + strftime(str_timebound, sizeof(str_timebound), "%F %T (UTC)", tm); + strlcpy(str_lines[1], str_timebound, sizeof(str_lines[1])); + } else { + strlcpy(str_lines[1], _("[no restriction]"), sizeof(str_lines[1])); + } - // Reset for timebound_max - memzero(str_timebound, sizeof(str_timebound)); + // Reset for timebound_max + memzero(str_timebound, sizeof(str_timebound)); - timebound = (time_t)msg->timebounds_end; - strlcpy(str_lines[2], _("Valid to:"), sizeof(str_lines[2])); - if (timebound) { - tm = gmtime(&timebound); - strftime(str_timebound, sizeof(str_timebound), "%F %T (UTC)", tm); - strlcpy(str_lines[3], str_timebound, sizeof(str_lines[3])); - } else { - strlcpy(str_lines[3], _("[no restriction]"), sizeof(str_lines[3])); - } + timebound = (time_t)msg->timebounds_end; + strlcpy(str_lines[2], _("Valid to:"), sizeof(str_lines[2])); + if (timebound) { + tm = gmtime(&timebound); + strftime(str_timebound, sizeof(str_timebound), "%F %T (UTC)", tm); + strlcpy(str_lines[3], str_timebound, sizeof(str_lines[3])); + } else { + strlcpy(str_lines[3], _("[no restriction]"), sizeof(str_lines[3])); } - if (msg->timebounds_start || msg->timebounds_end) { - stellar_layoutTransactionDialog(_("Confirm Time Bounds"), str_lines[0], - str_lines[1], str_lines[2], str_lines[3]); - if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) { - stellar_signingAbort(_("User canceled")); - return; - } + stellar_layoutTransactionDialog(_("Confirm Time Bounds"), str_lines[0], + str_lines[1], str_lines[2], str_lines[3]); + if (!protectButton(ButtonRequestType_ButtonRequest_ProtectCall, false)) { + stellar_signingAbort(_("User canceled")); + return; } } diff --git a/python/src/trezorlib/messages.py b/python/src/trezorlib/messages.py index 6e6d106d0..7304ceff7 100644 --- a/python/src/trezorlib/messages.py +++ b/python/src/trezorlib/messages.py @@ -5966,8 +5966,8 @@ class StellarSignTx(protobuf.MessageType): 4: protobuf.Field("source_account", "string", repeated=False, required=True), 5: protobuf.Field("fee", "uint32", repeated=False, required=True), 6: protobuf.Field("sequence_number", "uint64", repeated=False, required=True), - 8: protobuf.Field("timebounds_start", "uint32", repeated=False, required=False), - 9: protobuf.Field("timebounds_end", "uint32", repeated=False, required=False), + 8: protobuf.Field("timebounds_start", "uint32", repeated=False, required=True), + 9: protobuf.Field("timebounds_end", "uint32", repeated=False, required=True), 10: protobuf.Field("memo_type", "StellarMemoType", repeated=False, required=True), 11: protobuf.Field("memo_text", "string", repeated=False, required=False), 12: protobuf.Field("memo_id", "uint64", repeated=False, required=False), @@ -5982,11 +5982,11 @@ class StellarSignTx(protobuf.MessageType): source_account: "str", fee: "int", sequence_number: "int", + timebounds_start: "int", + timebounds_end: "int", memo_type: "StellarMemoType", num_operations: "int", address_n: Optional[List["int"]] = None, - timebounds_start: Optional["int"] = None, - timebounds_end: Optional["int"] = None, memo_text: Optional["str"] = None, memo_id: Optional["int"] = None, memo_hash: Optional["bytes"] = None, @@ -5996,10 +5996,10 @@ class StellarSignTx(protobuf.MessageType): self.source_account = source_account self.fee = fee self.sequence_number = sequence_number - self.memo_type = memo_type - self.num_operations = num_operations self.timebounds_start = timebounds_start self.timebounds_end = timebounds_end + self.memo_type = memo_type + self.num_operations = num_operations self.memo_text = memo_text self.memo_id = memo_id self.memo_hash = memo_hash