feat(cmd/network): add network reward set command

[Pantani]

close #2076
close #2077
close #2118

Description

Add the network chain reward set command to add the chain reward to the network as a coordinator.

How to test

• Publish and create a chain

starport n --local chain publish https://github.com/lubtd/earth --from alice && \
starport n --local chain init 1 --from alice --validator-account alice

• Set the chain reward

starport network --local reward set 1 1000 10uspn --from alice  

• or you can publish your chain directly with the rewards flags

starport n --local chain publish https://github.com/lubtd/earth --reward.coins 10uspn --reward.height 100 --from alice 

[ilgooz]

How can we revert adding rewards, making it like never set for all/some denoms?

[lumtis]

How can we revert adding rewards, making it like never set for all/some denoms?

Setting rewards is an idempotent operation. If the rewards for the chain are 50foo,50bar and you set it to 25foo, 25 foo and 50 bar will be refunded to you

There is the edge case of setting no reward to remove everything, I thought it was tracked in spn blockchain backlog but figured out that not, I created an issue for this.
In the client, we can eventually have a command reward remove to make it more explicit

[ilgooz]

As you have described, set can also remove tokens if you provide a smaller about than what was set before.

Creating a second remove command might be confusing for the user since it'll provide a similar but more complete functionality.

Instead, what do you think about allowing these syntaxes for set (given that 50foo,50bar initially set):

• set 50foo,40bar: 50foo,40bar
• set 50foo,0bar: 50foo
• set 50foo: 50foo
• set 0foo,0bar:
• set 0foo:
• set 0bar:
• set 0:

And maybe we rename chain reward set to chain reward-set

[ilgooz]

Or making it simpler, instead of above, just allow chain reward-set 0 to revert all.

[lumtis]

For remove, I was thinking of a command without argument except the launch ID, just a shortcut to delete the reward pool for the chain

And maybe we rename chain reward set to chain reward-set

In this case, it would rather be set-reward I think

For the reward namespace, it depends on what we want to propose to the user.
Currently, the command ask to provide the height where rewards are distributed (so length of the testnet basically) and the coins for the rewards distributed

We could have two commands reward set-coins and reward set-height to allow the user to update a single value

[ilgooz]

Is the height optional, what happens if I don't set it if we have reward set-coins, reward set-height? Do we use height 1 as default?

[lumtis]

Is the height optional, what happens if I don't set it if we have reward set-coins, reward set-height? Do we use height 1 as default?

No, both values should be defined when the reward pool is created. This would be some utility commands to make it easier to update a specific value. Setting the reward height could have more option also, like providing a time instead of a block height, 1 month -> x block estimated for the height

[ilgooz]

We may need to rethink both the --reward flag we planned to introduce to the chain publish cmd and rethink the chain reward commands to be able to support advanced use cases like you described.

The --reward flag on publish, by itself it won't be enough to be able to provide a height/time along with the coins within a single string format.

We may need to introduce sub flags like this:

• chain publish --reward --reward.coins 10uspn --reward.height 10
• chain publish --reward --reward.coins 10uspn --reward.time 1mo

If you use --reward but did not provide one of --reward.coins and --reward.time cmd should fail.

If you provide --reward.coins and --reward.time at the same time, publish cmd should fail.

And for the reward commands (this PR) it is also a bit confusing. Let's say that user didn't provide --reward flag during publish but wants to set it later through the reward commands.

In that case, we somehow need to disallow using chain reward set-coin by itself because we don't want to default height to a certain value (it was never set) and expect user to provide a height explicitly. So, we somehow need to encourage user to use chain reward set command instead if this her/his first time setting a reward for a launch. But this workflow a bit complicated.

Let's rethink the UX in all of these. Maybe it's better to allow defaults for simplicity or maybe we can introduce a reward-set (or set-reward) command in this structure:

• chain reward-set [launch-id] [coins] --height/--time

cc @fadeev

[lumtis]

The --reward flag on publish, by itself it won't be enough to be able to provide a height/time along with the coins within a single string format.
We may need to introduce sub flags like this:
chain publish --reward --reward.coins 10uspn --reward.height 10
chain publish --reward --reward.coins 10uspn --reward.time 1mo
If you use --reward but did not provide one of --reward.coins and --reward.time cmd should fail.
If you provide --reward.coins and --reward.time at the same time, publish cmd should fail.

For the publish command the reward pool will not be set, so coins and height must actually be defined at the same time

[Pantani]

Or making it simpler, instead of above, just allow chain reward-set 0 to revert all.

I agree to enable empty coins reward to remove all rewards

[Pantani]

We may need to rethink both the --reward flag we planned to introduce to the chain publish cmd and rethink the chain reward commands to be able to support advanced use cases like you described.

The --reward flag on publish, by itself it won't be enough to be able to provide a height/time along with the coins within a single string format.

We may need to introduce sub flags like this:

• chain publish --reward --reward.coins 10uspn --reward.height 10
• chain publish --reward --reward.coins 10uspn --reward.time 1mo

If you use --reward but did not provide one of --reward.coins and --reward.time cmd should fail.

If you provide --reward.coins and --reward.time at the same time, publish cmd should fail.

And for the reward commands (this PR) it is also a bit confusing. Let's say that user didn't provide --reward flag during publish but wants to set it later through the reward commands.

In that case, we somehow need to disallow using chain reward set-coin by itself because we don't want to default height to a certain value (it was never set) and expect user to provide a height explicitly. So, we somehow need to encourage user to use chain reward set command instead if this her/his first time setting a reward for a launch. But this workflow a bit complicated.

Let's rethink the UX in all of these. Maybe it's better to allow defaults for simplicity or maybe we can introduce a reward-set (or set-reward) command in this structure:

• chain reward-set [launch-id] [coins] --height/--time

cc @fadeev

This can be helpful to avoid one more command, but it's a little tricky for the user to know all flags and how to set them. Having one more command is more explicit

[fadeev]

chain publish --reward --reward.coins 10uspn --reward.height 10
chain publish --reward --reward.coins 10uspn --reward.time 1mo

This is too verbose. Height and time are the same things just measured in different mutually exclusive units. If height and time specify the span, during which rewards will be distributed, we should use that instead.

chain publish --reward.coins 10uspn --reward.duration 1mo
chain publish --reward.coins 10uspn --reward.duration 10blocks

[ilgooz]

Hey, it prints the following log twice:

gitpod /workspace/starport/.gitpod $ starport n --nightly reward set 4 0 10uspn 
𝓲 The reward pool was removed.
✔ The reward pool was removed.

[ilgooz]

I'm providing the both flags but still getting an error, can we please check that?

gitpod /workspace/starport/.gitpod $ starport n --nightly chain publish --chain-id mars-2 https://github.com/ilgooz/mars --reward.coins 11uspn --reward.height 30
reward.coins and reward.height flags must be provided together