Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Ensure SpuriousDragon transaction validation enforces chain_id match. #63

Closed
pipermerriam opened this issue Oct 3, 2018 · 5 comments
Closed

Comments

@pipermerriam
Copy link
Member

What is wrong?

It doesn't appear that transaction validation in the SpuriousDragon fork (or subsequent forks) implements validation for EIP155 transactions to ensure that the encoded chain_id matches the Chain.chain_id.

How can it be fixed

It looks like we'll need a new validation mechanism at the Chain level to do this enforcement. We currently have VM.validate_transaction_against_header and State.validate_transaction. I think we should add Chain.validate_transaction_against_chain.

It looks like this will need to be called in both MiningChain.apply_transaction and Chain.build_block_with_transactions

@pipermerriam
Copy link
Member Author

Testing for this should cover both cases, on both the SpuriousDragon fork and all subsequent forks.

@pipermerriam pipermerriam transferred this issue from ethereum/py-evm Dec 21, 2018
@carver
Copy link
Contributor

carver commented Jul 26, 2019

This is related to some work happening for EIP-1344 in py-evm. Time to discuss the right approach for threading the chain ID into the place it's needed (presumably ExecutionContext for 1344).

An idea from chat:

maybe a reasonable thing to do is to add a attribute like chaindb.chain_id and then pass that in as a new param to create_execution_context()

@fubuloubu
Copy link

I was working on a PR for 1344, got started here: https://github.com/fubuloubu/py-evm/tree/eip1344-chain-id

@carver
Copy link
Contributor

carver commented Jul 27, 2019

I was working on a PR for 1344, got started here: https://github.com/fubuloubu/py-evm/tree/eip1344-chain-id

Great!

Even better if you can open a PR with [WIP] in the title so we can see the diff. No need to worry about failing tests, dirty commits, etc. People know to ignore all that on a work-in-progress.

@fubuloubu
Copy link

PR: ethereum/py-evm#1803

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

No branches or pull requests

3 participants