Eth addresscase #107
Merged
Eth addresscase #107
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
asi345
force-pushed
the
eth_addresscase
branch
2 times, most recently
from
cb4eeda to
3bbbd0f
Compare
benma
requested changes
Aug 28, 2024
api/firmware/eth_test.go
Outdated
| recipient := [20]byte{0x04, 0xf2, 0x64, 0xcf, 0x34, 0x44, 0x03, 0x13, 0xb4, 0xa0, | ||
| 0x19, 0x2a, 0x35, 0x28, 0x14, 0xfb, 0xe9, 0x27, 0xb8, 0x85} | ||
| value := new(big.Int).SetUint64(530564000000000000) | ||
| data := []byte("") |
api/firmware/eth_test.go
Outdated
| recipient := [20]byte{0x04, 0xf2, 0x64, 0xcf, 0x34, 0x44, 0x03, 0x13, 0xb4, 0xa0, | ||
| 0x19, 0x2a, 0x35, 0x28, 0x14, 0xfb, 0xe9, 0x27, 0xb8, 0x85} | ||
| value := new(big.Int).SetUint64(530564000000000000) | ||
| data := []byte("") |
asi345
force-pushed
the
eth_addresscase
branch
2 times, most recently
from
2f3bb3e to
10ccf55
Compare
benma
requested changes
Aug 29, 2024
| @@ -141,6 +141,17 @@ func (device *Device) handleSignerNonceCommitment(response *messages.ETHResponse | |||
| return signature, nil | |||
| } | |||
|
|
|||
| func ETHIdentifyCase(recipientAddress string) messages.ETHAddressCase { | |||
There was a problem hiding this comment.
please add a docstring of the form // ETHIdentifyCase .... All exported API functions should come with docs.
| return messages.ETHAddressCase_ETH_ADDRESS_CASE_MIXED | ||
| } | ||
| } | ||
|
|
||
| // ETHSign signs an ethereum transaction. It returns a 65 byte signature (R, S, and 1 byte recID). |
There was a problem hiding this comment.
Add to the docstring what recipientAddressCase does and mention ETHIdentifyCase exists as a convenience function.
asi345
force-pushed
the
eth_addresscase
branch
from
10ccf55 to
cccb6e3
Compare
see : BitBoxSwiss/bitbox02-firmware@055bda6 For ETH signing legacy and EIP1559 transactions, the BitBox02 device shows confirmation screens for recipient and amount. For recipient, the device was previously showing the address in mixed case according to EIP-55, no matter the case user has entered. This created confusions among the users. That is why not the go library detects the case of the recipient address useer inputted and sends this case information to BitBox02 device, which will in turn show the recipient address in the same case that it got from the library. Signed-off-by: asi345 <inanata15@gmail.com>
There was no unit tests for these functionalities before. For any occasion, we added them so that modifications to these functions will be tested. Signed-off-by: asi345 <inanata15@gmail.com>
asi345
force-pushed
the
eth_addresscase
branch
from
cccb6e3 to
b8e2672
Compare
benma
reviewed
Aug 29, 2024
Comment on lines
+144
to
+148
| // ETHIdentifyCase identifies the case of the recipient address given as hexadecimal string. | ||
| // This function exists as a convenience to potentially help clients to determine the case of the | ||
| // recipient address. The output of the function goes to ETHSign and ETHSignEIP1559 as the | ||
| // recipientAddressCase parameter, which is forwarded to BitBox02 device to reconstruct the address | ||
| // in the correct case(the one that the user enters). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commits for detailed explanation.