cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.27k stars 3.63k forks source link

Combine redelegation and delegation commands #1293

Closed rigelrozanski closed 5 years ago

rigelrozanski commented 6 years ago

These commands are similar enough that they can be merged. we just need to explicitly specify the --source which could either be from liquid atoms or from a specific unbonding source - valid input should be:

CC: @jolesbi

cwgoes commented 6 years ago

Can you pull from multiple sources (delegating some unbonded Atoms and redelegating some already-bonded Atoms) in the same transaction or do we want to require separate transactions?

I think the latter is preferable, simpler and avoids the user using the wrong source of funds by accident.

rigelrozanski commented 6 years ago

Yeah I think we want separate transactions, if a UI figures out a way to make this non-ambiguous then the UI could choose to send multiple transactions with one click, but this logic doesn't need to exist in the back end

jbibla commented 6 years ago

@rigelrozanski what is your thinking on ...?

UNBONDED and BONDED LIQUID and NON-LIQUID DELEGATED and UNDELEGATED STAKED and UNSTAKED

so many options!

ValarDragon commented 6 years ago

I think liquid vs non-liquid is more confusing than the other options. (The other options are more directly related to the intended reason behind making the atoms non-liquid). That being said, adding aliases to support multiple of these shouldn't be hard either if we want to do that.

rigelrozanski commented 6 years ago

yeah liquid/nonliquid is very confusing, I'll try to refrain using those words. Let's also not add aliases. I'm trying to exclusively use the term delegation to describe the object which is holds the delegation information, and bonded / unbonding / unbonded to describe the state of a tokens. note that redelegating tokens are also bonded

jackzampolin commented 6 years ago

Have we made this change in some of the staking/slashing PRs? cc @cwgoes @rigelrozanski

rigelrozanski commented 6 years ago

No we have not - we still want this though! - although this doesn't block launch - changing labels

jackzampolin commented 5 years ago

Don't think this is pre-1.0. Removing label.

cwgoes commented 5 years ago

Not sure if this is desirable; at this point it might be confusing.

rigelrozanski commented 5 years ago

okay, I reckon we ought to close then.