8000 feat(network): `network rewards release` command by Pantani · Pull Request #2516 · ignite/cli · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(network): network rewards release command #2516

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 64 commits into from
Jul 29, 2022
Merged

feat(network): network rewards release command #2516

merged 64 commits into from
Jul 29, 2022

Conversation

Pantani
Copy link
Collaborator
@Pantani Pantani commented May 18, 2022 •
edited
Loading

close #2507

Description

implement ignite n rewards release

How to test

  • Run the SPN
ignite chain serve -v -r
  • Create and launch the chain into the network and accept all default values
ignite n --local chain publish https://github.com/Pantani/mars --reward.coins 333uspn --reward.height 120 --from alice && \
ignite n --local chain init 1 --validator-account alice --from alice && \
ignite n --local chain join 1 --amount 99998833339stake --from alice && \
ignite n --local chain launch 1 --from alice && \
ignite n --local chain prepare 1 --from alice && \
marsd start --home ~/spn/1
  • Connect relayer
ignite  n --local reward release 1 "http://0.0.0.0:26659" --from alice --testnet-account alice
  • If you don't want to connect, but only create the client id, you can use the --create-client-only flag
ignite  n --local reward release 1 "http://0.0.0.0:26659" --from alice --testnet-account alice --create-client-only

@Pantani Pantani self-assigned this May 20, 2022
@Pantani Pantani changed the title feat(network): network client connect command feat(network): network rewards release command May 23, 2022
@Pantani Pantani requested a review from ilgooz May 25, 2022 14:50
@Pantani Pantani requested a review from tbruyelle as a code owner July 26, 2022 15:50
@lumtis lumtis marked this pull request as draft July 27, 2022 08:11
@lumtis lumtis marked this pull request as ready for review July 27, 2022 08:28
@lumtis
Copy link
Contributor
lumtis commented Jul 27, 2022

Working with the following workflow:

Start spn

ignite chain serve -c config_2.yml -r

Testnet:

// publish with rewards
ignite n chain publish https://github.com/tendermint/spn --reward.coins 333uspn --reward.height 120 --spn-node-address http://0.0.0.0:26661/ --spn-faucet-address http://0.0.0.0:4502/ --chain-id spn-2

// init
ignite n chain init 1 --spn-node-address http://0.0.0.0:26661/

// join
ignite n chain join 1 --spn-node-address http://0.0.0.0:26661/ --amount 100000000stake

// trigger launch
ignite network chain launch 1 --spn-node-address http://0.0.0.0:26661/

// prepare
ignite network chain prepare 1 --spn-node-address http://0.0.0.0:26661/

// start
spnd start --home ~/spn/1

// rewards
ignite  n  reward release 1 "http://0.0.0.0:26657" --spn-node-address http://0.0.0.0:26661 --spn-faucet-address http://0.0.0.0:4502 --testnet-prefix spn

ignite  n  reward release 1 "http://0.0.0.0:26657" --spn-node-address http://0.0.0.0:26661 --spn-faucet-address http://0.0.0.0:4502 --testnet-prefix spn

𝓲 Network client: 07-tendermint-0
𝓲 Testnet chain spn-2 client: 07-tendermint-0

🔐  Account on "source" is default(spn165wt8n7n9280vqrgkjz85d4ew90fe3vt6cg8v6)

received coins from a faucet
 |· (balance: 199667uspn)

🔐  Account on "target" is default(spn165wt8n7n9280vqrgkjz85d4ew90fe3vt6cg8v6)

no faucet available, please send coins to the address
 |· (balance: -)

------
Paths
------

spn-1-spn-2:
    spn-1 > (port: monitoringc) (channel: channel-0)
    spn-2 > (port: monitoringp) (channel: channel-0)

------
Listening and relaying packets between chains...
------

Relay 1 packets from spn-2 => spn-1
Relay 1 acks from spn-1 => spn-2

spnd q monitoring-provider show-monitoring-info
MonitoringInfo:
  signatureCounts:
    blockCount: "119"
    counts:
    - RelativeSignatures: "119.000000000000000000"
      opAddress: spnvaloper165wt8n7n9280vqrgkjz85d4ew90fe3vtccy8cs
  transmitted: true

There are a lot of improvements for the commands and some bugs, like you can't rerun it if an error occurred for a chain (some stuff with the relayer config maybe)

But in the current state I would consider as a good initialization of the command
Most codes of the PR are in new files so it will be easy to get back to it once we work again on this feature

Open for review @ilgooz @tbruyelle @jeronimoalbi @aljo242

@tbruyelle
Copy link
Contributor
tbruyelle commented Jul 27, 2022 •
edited
Loading

@lubtd seems to work like a charm : the monitoring-info is no longer updated once block height 119 is reached.

$ spnd q monitoring-provider show-monitoring-info
MonitoringInfo:
  signatureCounts:
    blockCount: "119"
    counts:
    - RelativeSignatures: "119.000000000000000000"
      opAddress: spnvaloper13gdkcvyz8us4u5lwdh7n5adldydrzmmg4nh027
  transmitted: true

Some questions though :

  1. The reward distribution is not implemented yet right ? Only the number of signed blocks is recorded by the monitoring, so I suppose the distribution will be based on that ?

  2. The reward amount for a validator = rewardPool / rewardHeight * numSignedBlocks ?

  3. The white paper says the testnet chain imports monitoring-provider module, while the ignite chain imports the monitoring-consumer module. So I'm confused the command to read the monitoring info is spnd q monitoring-provider, giving the white paper I would expect spnd q monitoring-consumer, maybe there's something to fix somewhere ?

  4. Why both monitoring modules are imported in spn, only one is necessary right ?

  5. n reward release runs the relayer allowing the monitoring info exchange between the testnet chain and spn, but since it takes the address node as an argument, does that mean each validator will need to run this command to synchronize their monitoring info ?

@lumtis
Copy link
Contributor
lumtis commented Jul 27, 2022

The reward distribution is not implemented yet right ? Only the number of signed blocks is recorded by the monitoring, so I suppose the distribution will be based on that ?

It is implemented. The logic is handled in the reward module. You should be able to see that the reward pool is closed and the validator has uspn distributed on spn

The reward amount for a validator = rewardPool / rewardHeight * numSignedBlocks ?

It's a little bit more complicated. This is rewardPool multiply by the proportion of blocks signed by the validator, divided by the validator set size. This has not been put in the whitepaper yet, I can find for you some documentation for the formula

The white paper says the testnet chain imports monitoring-provider module, while the ignite chain imports the monitoring-consumer module. So I'm confused the command to read the monitoring info is spnd q monitoring-provider, giving the white paper I would expect spnd q monitoring-consumer, maybe there's something to fix somewhere ?

Correct, here I just use another spn network as a testnet as well. spn imports monitoring provider for test purposes

Why both monitoring modules are imported in spn, only one is necessary right ?

Test purposes, the module is inactive when non initialized

n reward release runs the relayer allowing the monitoring info exchange between the testnet chain and spn, but since it takes the address node as an argument, does that mean each validator will need to run this command to synchronize their monitoring info ?

The command just need to be run once by one entity, once the connection is performed, this is created on-chain, rpc URL is just to decide which node communicate to

685C
@lumtis
Copy link
Contributor
lumtis commented Jul 28, 2022

Thanks for the review @tbruyelle

@lumtis lumtis requested a review from tbruyelle July 28, 2022 14:24
tbruyelle
tbruyelle previously approved these changes Jul 28, 2022
View reviewed changes
Copy link
Contributor
@tbruyelle tbruyelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🥇

@ilgooz
Copy link
Member
ilgooz commented Jul 28, 2022

Thank you @Pantani and everyone who contributed!

@lumtis lumtis merged commit 64f5cb6 into develop Jul 29, 2022
@lumtis lumtis deleted the feat/network-connect branch July 29, 2022 07:59
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
@Pantani @ilgooz @lumtis
feat(network): network rewards release command (ignite#2516)
de34ff9 
* network connect cmd

* rly commands

* fetch client id from target chain

* change connect logic order

* find client state

* network connect

* remove comments and add TODO's

* fetch client id from spn

* improve the node client

* fecth client id from the monitoring client package

* avoid unecessary link

* fix get path from relayer

* add todo comment

* pass session to the nodetime call log

* change the client and channel queries

* update spn version

* update mock campaign queries

* remove session from tsrelayer pkg

* fix start relayer background job

* change create client command to reward release

* remove unused context InterfaceRegistry

* fetch spn chain id from status

* use the chain stake denom

* remove duplicated method

* fix channel id fetch for spn

* add validation for already established and is missing in one of the chains

* update spn

* make RPC port public

* make RPC port public

* convert node into a private var for network

* fix ibc-go version

* refactor: network release (ignite#2572)

* fix methods and var names

* fix import names

* errorIs

* remove unused id

* refactor create client

* simplify start

* import

* lint

Co-authored-by: İlker G. Öztürk <ilkergoktugozturk@gmail.com>
Co-authored-by: Petr Ivanov <petr.ivanov@tendermint.com>
Co-authored-by: ltacker <lucas.bertrand.22@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbruyelle tbruyelle tbruyelle approved these changes

@fadeev fadeev Awaiting requested review from fadeev

@jeronimoalbi jeronimoalbi Awaiting requested review from jeronimoalbi

@lumtis lumtis Awaiting requested review from lumtis

@ivanovpetr ivanovpetr Awaiting requested review from ivanovpetr

@ilgooz ilgooz Awaiting requested review from ilgooz

@aljo242 aljo242 Awaiting requested review from aljo242

Assignees

@Pantani Pantani

@lumtis lumtis

Labels
None yet
Projects
None yet
Milestone
Priority
Development

Successfully merging this pull request may close these issues.

cmd: implement ignite n rewards release
6 participants
@Pantani @fadeev @ilgooz @lumtis @ivanovpetr @tbruyelle
0