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

Custom LCD routes cannot be specified without changing sdk code #1081

Closed
UnitylChaos opened this issue May 29, 2018 · 6 comments
Closed

Custom LCD routes cannot be specified without changing sdk code #1081

UnitylChaos opened this issue May 29, 2018 · 6 comments

Comments

@UnitylChaos
Copy link
Contributor

UnitylChaos commented May 29, 2018

Currently the routes for the LCD server are specified by calling into a registration function in the modules:

keys.RegisterRoutes(r)
rpc.RegisterRoutes(ctx, r)
tx.RegisterRoutes(ctx, r, cdc)
auth.RegisterRoutes(ctx, r, cdc, "acc")
bank.RegisterRoutes(ctx, r, cdc, kb)
ibc.RegisterRoutes(ctx, r, cdc, kb)
return r
}

This doesn't allow adding routes without changing the code above. I'm trying to add an LCD interface for an example (#1061), and don't want to introduce a dependency from the LCD code to the example code. (#715)

Quick fix I worked up is to add a list of callback functions to the process, but I don't know if this is the appropriate style.

@cwgoes
Copy link
Contributor

cwgoes commented Jun 4, 2018

I'm not sure we have a good solution for this. The implicit assumption seems to be that SDK applications will copy the LCD code and adopt it to their application, adding/removing routes as necessary.

I think that's right, but perhaps we can expose a cleaner interface with which to do that, and an easy way to import the "default" LCD functionality and add custom routes.

@ebuchman
Copy link
Member

You definitely should not have to fork the SDK to build an app. So we need to fix this !!

@TanNgocDo
Copy link

How about this status ?

@ebuchman ebuchman added the T:Bug label Jun 30, 2018
@shanev
Copy link

shanev commented Aug 7, 2018

I am running into this issue now because I am building a standalone app with a vendored SDK instead of a fork.

@cwgoes I like the idea of importing the default LCD routes and adding custom routes.

@musnit
Copy link

musnit commented Aug 16, 2018

I have the same issue.

I think this relates to a bigger conversation around how there is no clean interface/specification between modules and an application.

For example, if there was a well defined RegisterRoutes interface and a way for an app to register a list of all modules that it is using, then the LCD could get the module list from the app and iterate over it, running the RegisterRoutes for each module. This would also be useful for things like RegisterWire and possibly even Keeper initialization, InitGenesis, WriteGenesis, getting module store key defaults, codespace defaults...

Reducing dependencies between modules and having a common layer of abstraction there should likely be part of this same conversation - related to (#1810)

@jackzampolin
Copy link
Member

So I've looked into this a bit more and tried a couple of approaches. The big issue that I'm running into right now is that a number of the modules require access to the keybase, and the location of the keybase is not set (by viper) until runtime. This causes the keys directory to be created in whatever folder the rest-server is run in. The only way to remedy this is, as @musnit suggests, is to define a clean interface between the modules and client software. So this issue is related to #2729. Going to rewrite that issue to reflect.

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

No branches or pull requests

8 participants