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

basecoin relay fixes #134

Closed
ebuchman opened this issue Jun 22, 2017 · 8 comments
Closed

basecoin relay fixes #134

ebuchman opened this issue Jun 22, 2017 · 8 comments
Assignees

Comments

@ebuchman
Copy link
Member

  • should be able to handle basecli style key
  • relay coin/amount should be configurable (or relay mechanics need to be changed to allow folks without coins to relay ... )
@ethanfrey
Copy link
Contributor

Easiest way to work with basecli style keys is to make it a basecli command.

Acutally, if you want users (non-full nodes) to run this, it makes sense in basecli.

@ethanfrey
Copy link
Contributor

You want relay to work like basecli works, but as part of basecoin?

If it is meant to be run by people without full nodes, then I would suggest this. If it is meant to be run on full-nodes, then I am not sure why it should use basecli stuff?

We could have a randomly generated key.json and run full-node if you want. I am happy to make it a client command as well. I just don't like a mix of both, which usually assumes client and server on one machine, which is a dev situation, but not common in the real-world (I think).

@ebuchman
Copy link
Member Author

My thinking was it should be in basecoin because its a long running process. But it needs access to a key, and we should have only one way we manage keys, ie. the basecli way ... Conflicted!

@ethanfrey
Copy link
Contributor

basecli has a proxy command, which is long-running and emulates the tendermint rpc, only validating all responses.

@ethanfrey
Copy link
Contributor

I need to improve the logging of these processes, but I actually think a local rpc server is the main use of basecli in the future, to provide security for a web ui.

@ethanfrey
Copy link
Contributor

I could make a third command, for long running client processes?

basecoin, basecli, baseproxy?

@ethanfrey ethanfrey added 0.7 and removed 0.6.1 labels Jun 26, 2017
@ethanfrey
Copy link
Contributor

Okay, after talking with bucky, we will:

  1. Stick with 2 binaries for the time being (one for client, one for servers), although he is still not convinced we can't do it with one. I think server is a validating or non-validating node on an always-connected VM in the cloud. Client is my laptop/phone/etc. with my private key on my physical hardware. Commands go with the binary that fits the context.

  2. There are lots of open questions if we allow multiple relays running in parallel. We should do a deeper refactor of this code before tackling that. As part of the bigger 0.7 release.

@ethanfrey ethanfrey self-assigned this Aug 4, 2017
@jaekwon jaekwon removed the 0.7 label Jan 14, 2018
@ebuchman
Copy link
Member Author

we'll revisit relay when we get IBC back on new sdk - but this should also be thought about in #324

liamsi added a commit to liamsi/cosmos-sdk that referenced this issue Jun 26, 2018
alexanderbez added a commit that referenced this issue May 26, 2022
* fix testing and protobuf related issues

* remove unnecessary changes in legacy migration tests

* remove extraneous comments

* add test for minselfdelegation

* fix: Ensure that MsgCreateValidator's accompanying delegation is at least the provided MinSelfDelegation (#132)

* add check to make sure validator object is only created if min delegation threshold is met

* fix check location and use standard error type

* fix logic for check

* add test for new check

* update test comments for better context

* add checks for global min self delegation and fix corresponding tests

* add migration code

* fix migration code tests

* fix staking client unit tests

* tidy up

* Apply suggestions from code review

Co-authored-by: Dev Ojha <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>

* apply changes suggested in review

* fixed edge case error

* lint++

* lint++

* Update x/staking/keeper/msg_server.go

* Update x/staking/keeper/msg_server.go

* Update x/staking/keeper/msg_server.go

Co-authored-by: Aleksandr Bezobchuk <[email protected]>

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Dev Ojha <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants