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

Advice on where to implement parse_raw_transaction() #3109

Open
coccoinomane opened this issue Sep 27, 2023 · 6 comments
Open

Advice on where to implement parse_raw_transaction() #3109

coccoinomane opened this issue Sep 27, 2023 · 6 comments

Comments

@coccoinomane
Copy link
Contributor

Hello,

ethers.js has a handy parseTransaction function to decode an RLP-serialized transaction. The function understands whether it is dealing with a legacy transaction or a post-EIP1559 one, and spits out the decoded transaction object accordingly. Here's a link to its documentation & source code.

Do we have a similar function in web3.py? I could not find one. Such function would be useful in serveral scenarios. One that comes to mind is in a web3.py middleware that logs all calls to eth_sendRawTransaction.

I would be happy to have a go at developing a parse_raw_transaction() in web3.py, but first I would like to ask two questions that a web3.py veteran might know the answers to:

  1. Has this function (or a similar one) already been implemented?
  2. If not, where would be the best place for it in web3.py?

Thank you,
Cocco

@fselmo
Copy link
Collaborator

fselmo commented Sep 27, 2023

Hey @coccoinomane. There's an example here using the TransactionBuilder class from the latest vm. The example shows ArrowGlacierTransactionBuilder but really any since London should work since transactions haven't yet changed since then. You could use from eth.vm.forks.shanghai.transactions import ShanghaiTransactionBuilder as TransactionBuilder, for example.

It would definitely be handy to abstract this logic for a user though. I wouldn't be opposed to a utility method that lives in, say, web3.utils that does maybe even exactly what that blog post does but abstracts it for a user and spits out the desired transaction dict. This could be updated to pull in the latest vm's TransactionBuilder each time we upgrade the py-evm version in web3.py.

@coccoinomane
Copy link
Contributor Author

coccoinomane commented Sep 27, 2023

Thanks @fselmo for the quick reply! A method in web3.utils makes a lot of sense.

I did not know of TransactionBuilder, it looks great.

py-evm is not a dependency of web3.py, is it? Then one would need to extract the relevant logic from LondonTransactionBuilder and condense it in a parse_raw_transaction() method in web3.utils.

The parse_raw_transaction() would return a TxData.

If you think this makes sense, I can have a go at it.

@fselmo
Copy link
Collaborator

fselmo commented Sep 27, 2023

Ah, I suppose it isn't a dependency. Only if you install the tester install extra or the full dev install extra (i.e. pip install "web3[dev]"). We should really think on this a bit. It would be nice to not have so much maintenance since blob transactions are just around the corner. If we could concentrate the logic in just one place and expose it via web3.py then the dev cost there is more manageable and would make more sense.

@coccoinomane
Copy link
Contributor Author

coccoinomane commented Sep 28, 2023

I see.

What if we followed the lead of ethers.js, and condensed the logic in a separate web3/_utils/raw_transactions.py module, with three decoding functions: parse_legacy(raw_tx), parse_2930(raw_tx) and parse_1559(raw_tx)? The entry point would be a parse(raw_tx) that switches to the right decode function based on the first byte of the raw transaction.

Each parsing function will just be defining a dict with the key & types of the transaction (the sedes) and then call the rlp.decode() function.

Do you reckon this solution would entail too much maintenance in the future?

coccoinomane added a commit to coccoinomane/web3client that referenced this issue Sep 29, 2023
coccoinomane added a commit to coccoinomane/web3client that referenced this issue Sep 29, 2023
@coccoinomane
Copy link
Contributor Author

coccoinomane commented Oct 3, 2023

PS: While we decide how to proceed, I have implemented decoding of raw transactions in web3cli:

Command:

w3 tx parse-raw-tx 0x02f86a0a75843b9aca0084412e386682520894240abf8acb28205b92d39181e2dab0b0d8ea6e5d6480c080a0a942241378fafc80670c6dde3c39ba5b1c4f992cd26fa263e7bbc253696c9035a008cf3399808cb511f0171be2ba766ea5c9d424a97dae3fb24685c440eeefc4fa

Result:

{
    "accessList": [],
    "blockHash": null,
    "blockNumber": null,
    "chainId": 10,
    "data": "0x",
    "from": "0x240AbF8ACB28205B92D39181e2Dab0B0D8eA6e5D",
    "gas": 21000,
    "gasPrice": null,
    "maxFeePerGas": 1093548134,
    "maxPriorityFeePerGas": 1000000000,
    "hash": "0x8631361df65445a40fc46cff4625a2c070e618733d9ebdf31a31535276225b85",
    "input": null,
    "nonce": 117,
    "r": "0xa942241378fafc80670c6dde3c39ba5b1c4f992cd26fa263e7bbc253696c9035",
    "s": "0x08cf3399808cb511f0171be2ba766ea5c9d424a97dae3fb24685c440eeefc4fa",
    "to": "0x240AbF8ACB28205B92D39181e2Dab0B0D8eA6e5D",
    "transactionIndex": null,
    "type": 2,
    "v": null,
    "value": 100
}

@fselmo
Copy link
Collaborator

fselmo commented Oct 3, 2023

parse_legacy(raw_tx), parse_2930(raw_tx) and parse_1559(raw_tx)? The entry point would be a parse(raw_tx) that switches to the right decode function based on the first byte of the raw transaction.

I think this is simple enough and would be quite useful. It would be nice to set a default formatter on the method as a kwarg as well that could be overridden if desired but, by default, would yield a transaction that is formatted in the same way as in other places in the library. Something like:

from eth_utils import apply_formatters_to_dict
from web3._utils.method_formatters import TRANSACTION_RESULT_FORMATTERS

def parse_transaction(signed_tx: HexBytes, formatter=TRANSACTION_RESULT_FORMATTERS): 
    # parse transaction
    tx_dict = ...
    
    return tx_dict if formatter is None else apply_formatters_to_dict(tx_dict)

Depending on how the parsing is done it might need a different formatter but the concept should be similar. Thoughts there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants