Skip to content

Conversation

gballet
Copy link
Member

@gballet gballet commented Sep 30, 2025

Uses the go module's replace directive to delegate keccak computation to precompiles.

This is still in draft because it needs more testing. Also, it relies on a PR that I created, that hasn't been merged yet.

@gballet gballet added the zk label Sep 30, 2025
Copy link
Member Author

Choose a reason for hiding this comment

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

note to self: this file should no longer be needed and, once I confirmed that, it should be removed.

@fjl
Copy link
Contributor

fjl commented Oct 1, 2025

So the approach here is, you want to make a custom version of the cmd/keeper module, where you replace the github.com/ethereum/go-ethereum/crypto package with another module that changes some functions. The replacement is only applied when compiling for ziren.

I don't think it's a solid approach, and it seems like misuse of the toolchain. If we want to make the keccak implementation replaceable, we need to add hooks across the codebase that allows us to set it.

@gballet gballet force-pushed the keeper-ziren-keccak-precompile branch from 4bea991 to cbf86b4 Compare October 1, 2025 15:44
@gballet
Copy link
Member Author

gballet commented Oct 1, 2025

It's a standard use of the toolchain, and is used all over the place. Please expand why you think it's not solid?
I can't predict what every platform will require, but those I did work on all share the same syscall approach so I'm confident I can do that for every single one of them.

On the other hand, creating hooks all over the place means a lot more code, longer function signatures, reduced readability and a larger diff. I'm not convinced this is a better approach.

@fjl
Copy link
Contributor

fjl commented Oct 2, 2025

What I meant by 'misuse' is the practice of replacing a package with a module. I'm not sure this is really a supported feature of the toolchain.

https://go.dev/ref/mod#go-mod-file-replace

A replace directive replaces the contents of a specific version of a module, or all versions of a module, with contents found elsewhere.
...
A require directive that refers to a replaced module version is also needed, either in the main module’s go.mod file or a dependency’s go.mod file.

So the thing that's being replaced must be a required module. If we want to use the module replacement approach as a way of replacing code in go-ethereum, we would probably need to extract the crypto bits into their own module first, then replace that module.

Your approach here also relies on defining a ziren-specific module file go.ziren.mod. You need this because the replace directive for ziren has to live somewhere, but you can't have it in go.mod because the replacement should not be active for all builds, you just want it for ziren. The problem is, this go.ziren.mod file is not recognized by the toolchain, and AFAIK there is no way to make the go tool aware of an alternate module file. The build process would have to perform the equivalent of

mv go.ziren.mod go.mod go mod tidy 

and then hope it builds. This is ugly because none of the usual methods for maintaining go.mod will work for updating go.ziren.mod, nor will go.sum be kept in sync with it.

Overall, I feel module replacements are not the right tool for the job here. We have two other options:

  • define hooks in EVM, core, etc. to override the hash function, or any other crypto function. This would be like a 'crypto factory' type of object that gets passed around.
  • integrate the precompiles into upstream go-ethereum with build tags.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 participants