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

Problem: New validator projection. Implement migration scripts and views [Part 2] #684

Open
wants to merge 24 commits into
base: feature/652-validator-delegation
Choose a base branch
from

Conversation

ysong42
Copy link
Contributor

@ysong42 ysong42 commented Feb 11, 2022

Solution: fix #665 #678

Changes

  • Implement views and migrations for new validator projection.
  • Extend the example app. Add new temporary APIs, to retrieve validators, delegations, unbondingDelegations and redelegations at specific height.

New Temporary APIs Examples

# Get Validators list
http://localhost:8080/api/test/validators?height=600000

# Get Specific validator
http://localhost:8080/api/test/validators/tcrocnclcons1rxvzs3kzdekrjazlshlua2v4pz24v0hpedgha2?height=150000

# Get Validator Delegations
http://localhost:8080/api/test/validators/crocncl149dyku4d4c36dmvvw583xqcflau3d9x303ffcj/delegations?height=4234440&limit=1000

# Get Validator UnbondingDelegations
http://localhost:8080/api/test/validators/crocncl149dyku4d4c36dmvvw583xqcflau3d9x303ffcj/unbonding_delegations?height=4234440&limit=1000

# Get Validator Redelegations
http://localhost:8080/api/test/validators/tcrocncl15mstquxa00lw5499tptsrw888tkw8dkt0c8xud/redelegations?height=2619910&limit=1000

# Get Delegator Delegations
http://localhost:8080/api/test/delegators/tcro1k4nz2yalsuzuzxexcxuughp33cfqls7e6utarx/delegations?height=150000

Performance and Correctness

Please refer to: #678 (comment)

extra_configs:
BridgePendingActivity:
this_chain_id: "testnet-croeseid-4"
this_chain_name: "Crypto.org-Chain"
counterparty_chain_name: "Cronos"
channel_id: "channel-131"
starting_height: 899374
ValidatorDelegation:
unbonding_time: "2419200000000000ns"
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest to change to ms / s

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh this is from Crypto.org genesis. I just copied from the genesis.

Later when we support proposals updaing params in this projection, we will remove this config.

extra_configs:
BridgePendingActivity:
this_chain_id: "testnet-croeseid-4"
this_chain_name: "Crypto.org-Chain"
counterparty_chain_name: "Cronos"
channel_id: "channel-131"
starting_height: 899374
ValidatorDelegation:
unbonding_time: "2419200000000000ns"
slash_fraction_double_sign: "0.050000000000000000"
Copy link
Contributor

Choose a reason for hiding this comment

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

May i know that are those slash fractions can be change by proposal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://crypto.org/explorer/proposals

You can check those proposals there. So there are different type of proposals. And there are module params that could be configured through gov module's proposal.

httpapi.Success(ctx, validator)
}

func (handler *ValidatorDelegation) ListValidator(ctx *fasthttp.RequestCtx) {
Copy link
Contributor

Choose a reason for hiding this comment

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

may i know what is the diff between this ListValidator and the List in Validator projection view?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is on http handler. It is for parsing the query params. Then triggered the under-lying view logic to interact with DB.

@@ -0,0 +1,13 @@
CREATE EXTENSION btree_gist;

CREATE TABLE view_vd_delegations (
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest to change it in completed form instead of vd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. And the similar places below

@@ -0,0 +1,8 @@
CREATE TABLE view_vd_evidences (
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@@ -0,0 +1,6 @@
CREATE TABLE view_vd_redelegation_queue (
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@@ -0,0 +1,19 @@
CREATE TABLE view_vd_redelegations (
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@@ -0,0 +1,6 @@
CREATE TABLE view_vd_unbonding_delegation_queue (
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@@ -0,0 +1,13 @@
CREATE TABLE view_vd_unbonding_delegations (
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@@ -0,0 +1,6 @@
CREATE TABLE view_vd_unbonding_validators (
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@@ -0,0 +1,28 @@
CREATE TABLE view_vd_validators (
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

return nil
}

func (view *DelegationsView) checkIfDelegationRecordExistByHeightLowerBound(row DelegationRow) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest IsExistedByLowerBoundHeight(validatorAddress, delegatorAddress string, height int64) (bool, error) for readability

Copy link
Contributor Author

@ysong42 ysong42 Feb 21, 2022

Choose a reason for hiding this comment

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

Thanks! Updated. I will use isExistedByLowerBoundHeight to keep the func as private.

return view.setDelegationRecordHeightUpperBound(row)
}

func (view *DelegationsView) setDelegationRecordHeightUpperBound(row DelegationRow) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

UpdateUpperBoundHeight

Copy link
Contributor Author

@ysong42 ysong42 Feb 21, 2022

Choose a reason for hiding this comment

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

Thanks, updated. I will use updateUpperBoundHeight, as I want to keep it as private.


return nil
}

func (view *RedelegationsView) checkIfRedelegationRecordExistByHeightLowerBound(row RedelegationRow) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest change toIsExistedByLowerBoundHeight(delegatorAddress, validatorSrcAddress, validatorDstAddress string, height int64) for readability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated.

return view.setRedelegationRecordHeightUpperBound(row)
}

func (view *RedelegationsView) setRedelegationRecordHeightUpperBound(row RedelegationRow) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

UpdateUpperBoundHeight

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated.

CREATE TABLE view_vd_unbonding_delegation_queue (
id BIGSERIAL,
completion_time BIGINT NOT NULL UNIQUE,
dv_pairs JSONB NOT NULL,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add some description for short formed column?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

CREATE TABLE view_vd_redelegation_queue (
id BIGSERIAL,
completion_time BIGINT NOT NULL UNIQUE,
dvv_triplets JSONB NOT NULL,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add some description for short formed column?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.


return nil
}

func (view *UnbondingDelegationsView) checkIfUnbondingDelegationRecordExistByHeightLowerBound(row UnbondingDelegationRow) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

similar suggestion here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated.

return view.setUnbondingDelegationRecordHeightUpperBound(row)
}

func (view *UnbondingDelegationsView) setUnbondingDelegationRecordHeightUpperBound(row UnbondingDelegationRow) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

similar suggestion here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated.

return nil
}

func (view *ValidatorsView) checkIfValidatorRecordExistByHeightLowerBound(row ValidatorRow) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

similar suggestion here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated.

return view.setValidatorRecordHeightUpperBound(row)
}

func (view *ValidatorsView) setValidatorRecordHeightUpperBound(row ValidatorRow) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

similar suggestion here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated.

}

func (view *ValidatorsView) FindByTendermintAddr(tendermintAddress string, height int64) (ValidatorRow, error) {
func (view *ValidatorsView) findBy(
Copy link
Contributor

Choose a reason for hiding this comment

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

is it intended to make it private?

Copy link
Contributor Author

@ysong42 ysong42 Feb 21, 2022

Choose a reason for hiding this comment

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

Yes. Users are expected to use FindByOperatorAddr and FindByConsensusNodeAddr to find a specific record.

The FindByOperatorAddr and FindByConsensusNodeAddr will invoke findBy internally. These two functions are mainly for readability. The real logic is in findBy.

FindByTendermintAddr is not used in implementation, so I simply remove it.

@ysong42 ysong42 requested a review from davcrypto February 21, 2022 08:28
@ysong42
Copy link
Contributor Author

ysong42 commented Feb 21, 2022

@davcrypto Thanks for the review! The code is updated! Feel free to have another review 🙏

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

Successfully merging this pull request may close these issues.

2 participants