search

concrete-eth / concrete-geth

Concrete is a framework for building application-specific rollups on the OP Stack
GNU Lesser General Public License v3.0
47 stars 19 forks source link

Concrete import errorfix #65

Open Josh-121 opened 5 days ago

therealbytes commented 5 days ago

Could you give a quick explanation of the changes you made? For instance, what is state_precompiles []state_PrecompileMap for?

Also, you don't need to open a new PR every time you make changes, just push to the same branch you are merging.

Josh-121 commented 5 days ago

Could you give a quick explanation of the changes you made? For instance, what is state_precompiles []state_PrecompileMap for?

Also, you don't need to open a new PR every time you make changes, just push to the same branch you are merging.

Alright. After changing concrete.PrecompileMap type to map[common.Address]interface{} in the function arguments at state.db, there were instances in some files where those functions took in Precompiles() as argument which led to type conflicts. So StatePrecompiles() is basically the same implementation of Precompiles() but returns a non conflicting type that is used in such instances

Josh-121 commented 5 days ago

For instance in core/chain_makers, you can find statedb.CommitWithConcrete( concreteRegistry.Precompiles(args),......)

Precompiles return type conflicts with the modified arg type of CommitWithConcrete. So StatePrecompiles is used here

therealbytes commented 4 days ago

Tests are still failing.

I don't think you should be adding a special method to the registry. You could implement methods for PrecompileMap and accept an interface in statedb.

Josh-121 commented 2 days ago

Tests are still failing.

I don't think you should be adding a special method to the registry. You could implement methods for PrecompileMap and accept an interface in statedb.

In what way?I get type conflicts working with Precompilemap that's why I had to make a special function that returns interface and added to the registry so it can be accessed by files that have type conflicts when using the statedb methods. It seemed to work on my end so I'd like to find out what went wrong if possible or if u can suggest another approach

therealbytes commented 2 days ago

TestE2EKkvPrecompile is failing. This likely means the state DB isn't being committed with the right precompiles. As I sad, my suggested approach is:

I don't think you should be adding a special method to the registry. You could implement methods for PrecompileMap and accept an interface in statedb.