Skip to content
This repository was archived by the owner on Apr 16, 2019. It is now read-only.

fixed rlp_length for chain_id > 255 #381

Merged
merged 2 commits into from
Jul 19, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion firmware/ethereum.c
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,11 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node)
rlp_length += rlp_calculate_length(1, tx_type);
}
if (chain_id) {
rlp_length += rlp_calculate_length(1, chain_id);
int length = 0;
if (msg->has_chain_id) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already in an if (chain_id). I think this nested if isn't necessary or even hurting.

length = chain_id < 0x80 ? 0: chain_id < 0x100 ? 1: chain_id < 0x10000 ? 2: chain_id < 0x1000000 ? 3 : 4;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

length should be 1 even for chain_id < 0x80. rlp_calculate_length will take care of the special case.

    length = chain_id < 0x100 ? 1 : chain_id < 0x10000 ? 2 : chain_id < 0x1000000 ? 3 : 4;

}
rlp_length += rlp_calculate_length(length, chain_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theoretically the second parameter should be the first byte of chain_id, i.e. chain_id >> (8*(length-1)). However practically, the first byte is only used if length is 1, so this works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. right :)

rlp_length += rlp_calculate_length(0, 0);
rlp_length += rlp_calculate_length(0, 0);
}
Expand Down