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

feat: distinct go modules for all state machine modules #3787

Closed
wants to merge 8 commits into from

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Aug 16, 2024

Unblocks #3729

Description

One of the issues encountered when working on the multi-app prototype is that protobuf definitions get registered multiple times (once per state machine imported into the multiplexer). This results in errors like:

panic: concrete type *types.MsgPayForBlobs has already been registered under typeURL /, cannot register *types.MsgRegisterEVMAddress under same typeURL. This usually means that there are conflicting modules registering different concrete types for a same interface implementation

To resolve, this PR introduces distinct Go modules per state machine module. This enables the v2 and v3 state machine to import the exact same module (i.e. github.com/celestiaorg/celestia-app/x/blob) which will result in just one proto type being registered per actual type.

@rootulp rootulp added the backport:v2.x PR will be backported automatically to the v2.x branch upon merging label Aug 16, 2024
@rootulp rootulp self-assigned this Aug 16, 2024
@rootulp rootulp requested review from cmwaters and rach-id and removed request for rach-id August 16, 2024 19:32
This was an attempt to fix test-fuzz but it didn't work. The fuzz test
in x/blob/types complains that an error code has already been
registered. Notably this error doesn't occur during make build, make
install, or runtime (i.e. ./scripts/single-node.sh).
@rootulp
Copy link
Collaborator Author

rootulp commented Aug 29, 2024

Discussed with Callum offline and closing this for now. As a prerequisite to this work we need to make all state machine modules not import app.

@rootulp rootulp closed this Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:v2.x PR will be backported automatically to the v2.x branch upon merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant