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

feat: decoding candidates for unverified contracts #1466

Merged
merged 4 commits into from
Mar 4, 2019

Conversation

zachdaniel
Copy link
Contributor

@zachdaniel zachdaniel commented Feb 20, 2019

Motivation

  • In order to show what function we suspect was called, we need to store and then list any potential contract ABI matches from other verified smart contracts. This will give users as much information as possible about unverified contract.

Changelog

Enhancements

  • Store any new method identifiers we receive.
  • List them as possibilities on transactions for unverified transactions.

screen shot 2019-02-27 at 11 36 02 am

@coveralls
Copy link

coveralls commented Feb 21, 2019

Pull Request Test Coverage Report for Build dcfff138-763e-440b-aea1-904ae556fca1

  • 19 of 32 (59.38%) changed or added relevant lines in 4 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 84.506%

Changes Missing Coverage Covered Lines Changed/Added Lines %
apps/explorer/test/support/factory.ex 1 2 50.0%
apps/explorer/lib/explorer/chain/contract_method.ex 12 15 80.0%
apps/explorer/lib/explorer/chain/transaction.ex 2 6 33.33%
apps/explorer/lib/explorer/chain/smart_contract.ex 4 9 44.44%
Files with Coverage Reduction New Missed Lines %
apps/indexer/lib/indexer/block/catchup/fetcher.ex 2 66.67%
Totals Coverage Status
Change from base Build 1cdb12bf-a042-4b41-9afc-47bd5e6bef67: 0.0%
Covered Lines: 4276
Relevant Lines: 5060

💛 - Coveralls

@acravenho
Copy link
Contributor

I'm not able to get this to work correctly. I deployed and verified a simple token here: https://blockscout.com/poa/core/address/0x8deb2e426835067f48ac034958cdd9daa38bd0a2/contracts

If you want to replicate this locally the only thing extra you might need is the constructor arguments:

00000000000000000000000057da3f24d81186b344649fe92b120ad45100d749

This contract has a simple function

{"name": "transfer", "type": "function", "inputs": [{"name": "_to", "type": "address"}, {"name": "_value", "type": "uint256"}], "outputs": [{"name": "", "type": "bool"}], "payable": false, "constant": false, "stateMutability": "nonpayable"}

which is used by most other token transfers. They have the same 4byte signature.

I viewed a transaction with the same 4byte signature but there was no decoding of this input.

0x9f6079797ff552b661944d2fbaaa7b394b872b6d06254810a8fe2667e2e833e8

@acravenho
Copy link
Contributor

It also used an identifier of -145924998. I'm not sure where this comes from.

@zachdaniel
Copy link
Contributor Author

I'll take a look now.

@zachdaniel
Copy link
Contributor Author

The error can be shown just w/in the ex_abi library in a console like so:

selector = %ABI.FunctionSelector{
  function: "transfer",
  input_names: ["_to", "_value"],
  inputs_indexed: nil,
  method_id: <<169, 5, 156, 187>>,
  returns: [:bool],
  type: :function,
  types: [:address, {:uint, 256}]
}

input = <<159, 96, 121, 121, 127, 245, 82, 182, 97, 148, 77, 47, 186, 170, 123, 57, 75, 135, 43, 109, 6, 37, 72, 16, 168, 254, 38, 103, 226, 232, 51, 232>>

ABI.decode(selector, input)

@ayrat555 would you mind taking a look also if you get the chance?

@zachdaniel zachdaniel force-pushed the lookup-by-method-id branch 2 times, most recently from ce44dbd to 2e70958 Compare February 27, 2019 16:11
Copy link
Contributor

@acravenho acravenho left a comment

Choose a reason for hiding this comment

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

After playing with some contracts, I think it looks really good. The only change I would request is the danger to info for unverified contracts.

Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

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

Looks goot to me. Just some minor UI suggestions:

53506419-ecd4b300-3a83-11e9-89dc-a5d0b38eb735 4

But I do not insist on these UI changes. There is more important, that functionality works. I've tested it with some ERC20, ERC721 smart-contracts

@ghost ghost assigned vbaranov Mar 1, 2019
@vbaranov
Copy link
Member

vbaranov commented Mar 2, 2019

But I do not insist on these UI changes.

Oh, as I see we already use this UI for displaying of decoded inputs for verified contracts. Thus, forget my comment about UI 😃.

@vbaranov vbaranov merged commit e0ff08a into master Mar 4, 2019
@ghost ghost removed the in progress label Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants