search

ethereum / remix-project

Remix is a browser-based compiler and IDE that enables users to build Ethereum contracts with Solidity language and to debug transactions.
https://remix-ide.readthedocs.io
MIT License
2.47k stars 952 forks source link

Warn if hash collision in function signature #146

Closed yann300 closed 2 years ago

yann300 commented 6 years ago

Upon compilation complete, use https://www.4byte.directory/docs/ to resolve function signature, display a warning if there is collision (more than 1 result for a single signature hash)

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 0.4 ETH (187.4 USD @ $468.49/ETH) attached to it.

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Workers have applied to start work.

These users each claimed they can complete the work by 4 weeks from now. Please review their questions below:

  1. jvmaia has applied to start work _(Funders only: approve worker | reject worker)_.
    • Q: Read the actual code and ask any questions if I have, then start the work
gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 7 months, 4 weeks ago. Please review their action plans below:

1) jvmaia has been approved to start work.

Read the actual code and ask any questions if I have, then start the work

Learn more on the Gitcoin Issue Details page.

jvmaia commented 6 years ago

Let me see if I understood it right. When the compilation is completed, I will search at the https://www.4byte.directory/api/v1/signatures/?hex_signature=0x..... for each function, using it signature to check if I receive more than one result, if yes I will create a warn in the panel. Is it right? I have a doubt: where should I do this, inside the compile-tab.js, after call the parseContract()?

ligi commented 6 years ago

for each function, using it signature to check if I receive more than one result, if yes I will create a warn in the panel.

No you should already print the warning if there is only one result - the function in the contract would be the second then.

You can even have this easier - just check the URL

https://raw.githubusercontent.com/ethereum-lists/4bytes/master/signatures/<4byte_signature_lower_case>

if it returns 200 -> print the warning if it returns 404 -> don't print a warning

This not only makes it easier - but also might help make it more decentralized in the future as we can put the same structure on swarm when it is ready ..

jvmaia commented 6 years ago

thanks for the tips, I'll do it

ligi commented 6 years ago

Happy coding!

@yann300 - just wonder if this perhaps should be feature in a solidity linter and then just be consumed by remix - would feel a bit cleaner and more generally usable

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 0.4 ETH (206.39 USD @ $515.97/ETH) has been submitted by:

  1. @jvmaia

@vs77bb please take a look at the submitted work:

yann300 commented 6 years ago

More insight for this issue (please @ligi correct me if i m wrong) The warning should be displayed:

The text should display: "4bytes has recorded a hash collision in the function signature : " . This means that services using 4bytes will not be able to resolve to a single function signature and thus displaying correct information. You may want to update the function name in order to avoid this collision."

ligi commented 6 years ago

if there's more than 1 result

I think this does not really matter - you mainly want to know if you introduce a collision

if there's 1 result and if the function signature is not the same as the current function being checked

yes correct

"4bytes has recorded a hash collision in the function signature : " . This means that services using 4bytes will not be able to resolve to a single function signature and thus displaying correct information. You may want to update the function name in order to avoid this collision."

sound good

yann300 commented 6 years ago

ah ok I see now. My point was that If there is already a collision , perhaps the contract developer want to change the function signature so it is usable by 4bytes consumers.

ligi commented 6 years ago

ACK - I see

jvmaia commented 6 years ago

"4bytes has recorded a hash collision in the function signature : " . This means that services using 4bytes will not be able to resolve to a single function signature and thus displaying correct information. You may want to update the function name in order to avoid this collision."

Should I update the actual message?

yann300 commented 6 years ago

yes please and also could you take care of the previous messages: thanks :)

The warning should be displayed:

  • if there's more than 1 result
  • if there's 1 result and if the function signature is not the same as the current function being checked
jvmaia commented 6 years ago

Updated! I added '\n' in the message to be more readable.

vs77bb commented 6 years ago

@jvmaia Paying this out now per confirmation from @yann300 via Gitter! Great work 🙂

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

The funding of 0.4 ETH (184.67 USD @ $461.68/ETH) attached to this issue has been approved & issued to @jvmaia.

jvmaia commented 6 years ago

Thanks guys! =)