Helper for composing tx decoders

[notatestuser]

Summary

Feature request: Add a helper for composing tx decoders

Problem Definition

Right now we can give baseapp a single decoder like so:

NewBaseApp(name, logger, db, txDecoder(cdc))

but it would be great if we could compose multiple decoders together:

NewBaseApp(name, logger, db, sdk.ComposeTxDecoders(cdc, txDecoderOne, txDecoderTwo, ...))

not sure what kind of magic would exist in this method but we should be able to add and remove decoders without affecting the implementations of the others.

This would make it easier to include the decoders in x plugins to plug them in as new tx types are coded up.

Disadvantages: Breaks existing API? maybe not depending on implementation

Proposal

NewBaseApp(name, logger, db, sdk.ComposeTxDecoders(cdc, txDecoderOne, txDecoderTwo, ...))

or

NewBaseApp(name, logger, db, cdc.ComposeTxDecoders(txDecoderOne, txDecoderTwo, ...))

This would require implementing ComposeTxDecoders and handling it

Later thought: There might be two behaviours here - one that "waterfalls" down from the top passing the result of each decoder to the next one in the list and another that tries each decoder until one returns a non-nil tx.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[alexanderbez]

There might be two behaviours here - one that "waterfalls" down from the top passing the result of each decoder to the next one in the list and another that tries each decoder until one returns a non-nil tx.

Seems a bit dangerous here. There is a now an options argument you can pass to the app. Perhaps if/when we need this in the future, one of these options could be to "register" a tx decoder for a particular tx. That way a lookup could be done based on the tx.

[ValarDragon]

This sort of composition doesn't seem like something we should apply globally. I think its better if the app just writes the custom composition logic they expect. (The cases where you want multiple tx decoders seems rare to me. We're working on an EVM module, so that decoding stuff will be handled within that module)

[notatestuser]

Currently have some txs coming through in a non standard format to test the performance implications. So I just use the multiple decoders to handle these. I suppose it’s not going to be a really common thing but wanted to suggest this anyway.

To address the other comment above the decoder sig could be changed so that there is a “next” function to pass control down to the next decoder in the stack. This avoids the need for a type to decoder mapping and that would work more like middleware.

[ValarDragon]

I do think your right, there is a reasonable way to cascade this. I'm just not sure if this is api which is common enough to justify inclusion in every app. I could totally see having a separate module down the line for "baseapp helper methods", which aren't used in the default modules. (For cases like this where we don't expect many people to use it)

[jackzampolin]

We currently only have 1 transaction decoder in the SDK and have not seen a feature request for additional decoders. I'm going to close this issue as out of scope.