ethereum / web3.py

A python interface for interacting with the Ethereum blockchain and ecosystem.
http://web3py.readthedocs.io
MIT License
5.01k stars 1.71k forks source link

Contract decode_function_input #1631

Closed rymkapro closed 2 years ago

rymkapro commented 4 years ago

What was wrong?

Hi, I'm trying to decode contract function input (Tether). I've created contract with Tether contract address and Tether ABI.

contract = web3.eth.contract('tether_address', abi=tether_abi)
decoded_input = contract.decode_function_input(transaction['input'])

Mostly everything is ok, but there are some transactions, where I get exception eth_abi.exceptions.NonEmptyPaddingBytes: Padding bytes were not empty: b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x03&'

Here is such transaction example: https://etherscan.io/tx/0xf32e5a06a8e2929a6b0ae0d5caa423e51340e5fefa1a03def91979f01d063ed5

Thanks for reply!

kclowes commented 4 years ago

Thanks for the report! It does look like there may be a bug in our decoder, but I'm not 100% sure what is going on there. Can you provide any other transaction examples where you've seen the error?

rymkapro commented 4 years ago

I have only one more example for such tx: https://etherscan.io/tx/0x34204b3ef07a82b2c4a6e159612652ec03b793ca0d836bf471acd80ffdc81150

Also I've noticed, that etherscan.io also can't decode this wrong transactions' data, e.g. you can press "Decode Input Data" button on the etherscan tx page and there will be no result..

kclowes commented 4 years ago

Yeah, I noticed that too. It looks like it's a problem with the address parsing. I found a good one here, that has 24 leading 0s, which seems correct. The two examples that are raising the error in the decoder have less than 24 leading zeros. I will have to dig in a little more to figure out where the "bad" leading bits are coming from. I can't tell if it's something the Tether contract is adding that web3 should be able to handle, or if web3 is doing the right thing in throwing the error.

@pipermerriam do you have any ideas here?

Enigmatic331 commented 4 years ago

Was doing some studying on abi-decoding, and stumbled here; Thought I could chime in a bit.

So what likely happened to this tx (https://etherscan.io/tx/0x34204b3ef07a82b2c4a6e159612652ec03b793ca0d836bf471acd80ffdc81150) is that the input data was sent wrong. The "address" part of the input parameter probably was entered wrong (0a6a70bb27128702777f3996fcf451c36de563cedf57926753f4977318cb09fa) and the target address probably shouldn't have been 0xfcf451c36de563cedf57926753f4977318cb09fa even if the EVM just takes the required 40 to form an address.

So the error "padding bytes not empty" kinda makes sense, to me at least.

Digging a bit more, it does look like the sender is some sort of funding address (e.g. 0.01 being sent to an address here https://etherscan.io/address/0xd61c1574edf4b3422a364086e07fc62d150538b7) to user wallets so a particular ERC20 token could be withdrawn (and consolidate to a hot/cold wallet, like what some exchanges do).

So likely the tx highlight here is an outlier, and a programming mistake which costs 286USD.... 😢

pipermerriam commented 4 years ago

Our ABI library is very strict. There are going to be things that actually work in the EVM that fail our decoder. In the main example I believe we're looking at an ABI encoded address which only has 20 bytes of data and then 12 bytes of padding. Our decoder strictly enforces that these padding bytes are zero'd. However, smart contracts likely just read those 20 bytes since validating that the other 12 or zero'd would cost extra gas.

I haven't dug into the details of this so I'm not sure this is what is happening here but it seems likely related.

lvalladares commented 3 years ago

Hey! This keeps happening, would be nice to have a way to make a more lenient check, maybe a param on the function.

Here is the issue reported on eth-abi repo and their response: https://github.com/ethereum/eth-abi/issues/116

fselmo commented 2 years ago

@lvalladares I'm curious what a lenient check looks like for you. These are values that were not padded appropriately that neither etherscan nor the library can parse so this seems to be the desired behavior. I'd imagine one can try / except these particular exception cases if they want leniency around this, no? The conversation you linked from eth-abi seems to reach the same consensus.

I'm going to close this due to staleness but please feel free to ask to re-open if you believe there is something else related to web3.py that could be addressed related to this. All conversations encouraged.