Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better display ethereum function calls #69

Open
ligi opened this issue Apr 14, 2018 · 14 comments
Open

Better display ethereum function calls #69

ligi opened this issue Apr 14, 2018 · 14 comments
Assignees
Labels
altcoin Any non-Bitcoin coins core Trezor Core firmware. Runs on Trezor Model T and T2B1. feature Product related issue visible for end user

Comments

@ligi
Copy link
Contributor

ligi commented Apr 14, 2018

Currently when calling ethereum functions you only see the raw hex data - also often you do not see all of it as the hex gets long fast and I was not able to scroll.
The hex can be easily made shorter and more digestible by humans by resolving the function from the 4byte signature - e.g. via https://github.com/ethereum-lists/4bytes and then displaying the function name with parameters. So a really long hex string that might not fit on the trezor and is hard to parse by a human can easily fit on the display of the trezor and better digested by humans.
A problem might be the size of the signature database - I see 2 solutions here:

  • leverage the SD-Card
  • reduce the database to commonly used signatures only - we can parse all transactions currently done on chain - sort the 4bytes by usage and define a cut-off point
@prusnak
Copy link
Member

prusnak commented Apr 23, 2018

I am planning to create a mechanism which will allow a wallet to upload a signed definition of {coin, token, 4byte-function} as a part of signed transaction.

We would need to create a signed database where each row is signed by trusted key (public key of this keypair will be part of the Trezor firmware) and TREZOR will parse and check the signature of provided data-row.

That way, we would not need to store all coins/tokens/4bytes in the firmware.

@ligi
Copy link
Contributor Author

ligi commented Apr 23, 2018

That is a great idea! Please keep me updated on this

@prusnak
Copy link
Member

prusnak commented May 3, 2018

I was looking at the catalog and I am quite disappointed about the very high number of collisions in the space. It is so high, I have my doubts about the usefulness of the feature.

How do you solve the collision problem? Do you show all possible options?

@ligi
Copy link
Contributor Author

ligi commented May 3, 2018

I do not see many collisions - I was expecting more and was surprised that there where not too many collisions. In the 23504 signatures I parse currently there are 3 collisions:

aaaaaad1 yycU4()
aaaaaad1 uYZeB()

00000000 get_block_hash_257335279069929(uint256)
00000000 overdiffusingness(bytes,uint256,uint256,uint256,uint256)
00000000 left_branch_block(uint32)

0000006e bright_peace(bytes32,bytes)
0000006e display_allow(uint256,uint256,uint256,uint32)

the first 2 (aaaaaad1 and 00000000) sound really constructed and I do not think there is real world impact with these. And even 6e can IMHO be ignored. Wonder where your observation of :

high number of collisions in the space

is coming from.

@prusnak
Copy link
Member

prusnak commented May 3, 2018

is coming from.

I looked at 00000000 and expected the rest is the same :-)

@prusnak
Copy link
Member

prusnak commented May 3, 2018

Do you think it's possible to create something like a curated list where all collisions are resolved? That means no signature collisions (i.e. no entries for 00000000, 0000006e, etc.) and also where you pick the better value for a9059cbb from transfer(address _to,uint256 _value);transfer(address to,uint val)? Or this is just too much effort?

@ligi
Copy link
Contributor Author

ligi commented May 3, 2018

hehe - no - don't judge a list by the first entry ;-)

I think 00000000 is constructed - some context:
pipermerriam/ethereum-function-signature-registry#30 (comment)
http://swende.se/blog/Blockhash-Refactor.html

@ligi
Copy link
Contributor Author

ligi commented May 3, 2018

I think currently it is possible to curate a list that resolves the collisions - but not sure how future compatible this is ..

@prusnak
Copy link
Member

prusnak commented May 3, 2018

I will check how many collisions there will be left if I treat arguments _to and to like they are the same. Also maybe creating a small dictionary which says that val and value are the same could help.

Another question: is uint and uint256 the same?

@ligi
Copy link
Contributor Author

ligi commented May 3, 2018

Another question: is uint and uint256 the same?

yes - uint must be resolved to uint256 for the function selector

uint, int: synonyms for uint256, int256 respectively. For computing the function selector, uint256 and int256 have to be used.

https://solidity.readthedocs.io/en/develop/abi-spec.html

PS: the collisions I showed above are without parameter names - with parameter names there are more - currently I am not sure about the parameter names - hope to be able to discuss this in about 2 weeks in person with @holiman

@prusnak prusnak transferred this issue from trezor/trezor-core Apr 17, 2019
@prusnak prusnak added this to the backlog milestone Apr 17, 2019
@prusnak prusnak added core Trezor Core firmware. Runs on Trezor Model T and T2B1. and removed core Trezor Core firmware. Runs on Trezor Model T and T2B1. labels Apr 17, 2019
@ZdenekSL ZdenekSL modified the milestones: backlog, 2019-Q4 Jun 25, 2019
@tsusanka tsusanka modified the milestones: 2019-Q4, backlog Sep 20, 2019
@ZdenekSL ZdenekSL added the W? label Oct 24, 2019
@matejcik
Copy link
Contributor

linking to #15

@tsusanka tsusanka added the feature Product related issue visible for end user label Oct 29, 2020
@tsusanka tsusanka removed W? labels Feb 19, 2021
@tsusanka tsusanka moved this from 📥 Inbox to 📽 Product in Firmware · Backlog 🗂 Oct 5, 2021
@tsusanka tsusanka removed this from the backlog milestone Oct 6, 2021
@alex-jerechinsky alex-jerechinsky added this to 📽 Product in Backlog 🗂 Oct 22, 2021
@alex-jerechinsky alex-jerechinsky removed this from 📽 Product in Firmware · Backlog 🗂 Oct 22, 2021
@hynek-jina hynek-jina removed the LOW label Dec 17, 2021
@sime sime added the LOW label Feb 21, 2022
@hynek-jina hynek-jina removed the LOW label May 6, 2022
@Hannsek
Copy link
Contributor

Hannsek commented Mar 17, 2023

duplicate? #2874

@matejcik
Copy link
Contributor

yes, this has more info so i suggest closing #2874 and keeping this one

@Hannsek
Copy link
Contributor

Hannsek commented Mar 17, 2023

Copied from #2874:

Every transaction in Ethereum can carry additional input data (e.g. 0x095ea7b30000000000000000000000008bc3702c35d33e5df7cb0f06cb72a0c34ae0c56f00000000000000000000000000000000000000000000000ee5c13efe85190000) (https://etherscan.io/tx/0x2b930225479934eda949c3c2b0f3af5d5fd60136f7c9f0d5bbabf569def1f8a8). The first 4 bytes of this input data, i.e. 0x095ea7b3, specify which function in the smart contract gets executed. These 4 bytes are not human readable. Therefore, there exists signature databases which help to map the hash (4 bytes) to human-readable form.

Signature databases:

How to read data

example

0x38ed1739000000000000000000000000000000000000000000000000000000009502f900000000000000000000000000000000000000000000a07e38bf71936cbe39594100000000000000000000000000000000000000000000000000000000000000a00000000000000000000000003c02cebb49f6e8f1fc96158099ffa064bbfee38b00000000000000000000000000000000000000000000000000000000616e11230000000000000000000000000000000000000000000000000000000000000003000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2000000000000000000000000528b3e98c63ce21c6f680b713918e0f89dfae555
  • 0x - indicator that the hexadecimal
  • 38ed1739 - hashed signature of the function being called (every 2 hex characters represent a byte)
  • The rest of the bytes are hashes of the arguments being passed to the function

decoded data:

function called:  swapExactTokensForTokens
arguments:  {
  "amountIn": 2500000000,
  "amountOutMin": 194024196127819599854524737,
  "path": [
    "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48",
    "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2",
    "0x528B3e98c63cE21C6f680b713918E0F89DfaE555"
  ],
  "to": "0x3c02cebB49F6e8f1FC96158099fFA064bBfeE38B",
  "deadline": 1634603299
}

Possible next steps:

  • preload the well known smart contract addresses → label it → show the label next to the address (otherwise label it as unknown)
  • label the Smart Contract address user is authorising and highlight an unknown address to give an alert
  • address after parameter “to” (most of the time same as address of the sender) - compare “to” address with “from” address → if not same → warning
  • warn before signing an unlimited allowance contract https://satoshilabs.slack.com/archives/C03SRP8PSS2/p1680176003637509

@Hannsek Hannsek added the altcoin Any non-Bitcoin coins label Mar 17, 2023
cbergqvist added a commit to cbergqvist/trezor-firmware that referenced this issue Jul 25, 2023
cbergqvist added a commit to cbergqvist/trezor-firmware that referenced this issue Jul 25, 2023
Step toward solving issue trezor#69

Improved Ethereum address display and changed test cases to use addresses with valid checksums.

[no changelog]
cbergqvist added a commit to cbergqvist/trezor-firmware that referenced this issue Jul 26, 2023
Step toward solving issue trezor#69

Improved Ethereum address display and changed test cases to use addresses with valid checksums.

Added test for known ERC-20 token (DAI)

[no changelog]
cbergqvist added a commit to cbergqvist/trezor-firmware that referenced this issue Jul 26, 2023
Step toward solving issue trezor#69

Improved Ethereum address display and changed test cases to use addresses including letter to see the upper/lower case formatting.

Added test for known ERC-20 token (DAI)

[no changelog]
cbergqvist added a commit to cbergqvist/trezor-firmware that referenced this issue Jul 26, 2023
Step toward solving issue trezor#69

Improved Ethereum address display and changed test cases to use addresses including letter to see the upper/lower case formatting.

Added test for known ERC-20 token (DAI)

[no changelog]
cbergqvist added a commit to cbergqvist/trezor-firmware that referenced this issue Jul 26, 2023
Step toward solving issue trezor#69

Improved Ethereum address display and changed test cases to use addresses including letter to see the upper/lower case formatting.

Added test for known ERC-20 token (DAI)

[no changelog]
@obrusvit obrusvit self-assigned this Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
altcoin Any non-Bitcoin coins core Trezor Core firmware. Runs on Trezor Model T and T2B1. feature Product related issue visible for end user
Projects
Status: 🎯 To do
Backlog 🗂
📽 Product
Development

No branches or pull requests

10 participants