Update validator table indexes.

[dzmitryhil]

Remove validator_info_operator_address_index and validator_info_self_delegate_address_index from validator_info table since it is possible to have them duplicated.

---

This change is 

[wojtek-coreum]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dzmitryhil, @keyleu, @miladz68, and @ysv)

a discussion (no related file):
I don't understand the purpose of this PR. This are not unique indexes so why do we care about duplicates?

---

[dzmitryhil]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)

a discussion (no related file):

Previously, wojtek-coreum (Wojtek) wrote…

I don't understand the purpose of this PR. This are not unique indexes so why do we care about duplicates?

Right, updated.

---

[ysv]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil, @keyleu, @miladz68, and @wojtek-coreum)

---

database/schema/03-staking.sql line 29 at r2 (raw file):

(
    consensus_address     TEXT   NOT NULL UNIQUE PRIMARY KEY REFERENCES validator (consensus_address),
    operator_address      TEXT   NOT NULL,

so bdjuno doesn't have migrations define ?

How Artem Devops will apply this change ?

[wojtek-coreum]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil, @keyleu, and @miladz68)

---

database/schema/03-staking.sql line 29 at r2 (raw file):

(
    consensus_address     TEXT   NOT NULL UNIQUE PRIMARY KEY REFERENCES validator (consensus_address),
    operator_address      TEXT   NOT NULL,

Why operator address is not unique? In Cosmos SDK, one operator may create single validator only.

[dzmitryhil]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @keyleu, @miladz68, and @wojtek-coreum)

---

database/schema/03-staking.sql line 29 at r2 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

Why operator address is not unique? In Cosmos SDK, one operator may create single validator only.

Actually not.

Based on the history we can see that the delegator created 2 validators: https://explorer.testnet-1.coreum.dev/coreum/accounts/testcore1w3lry34l3j8p94ktvzm5smp45q9mqm206ewvj8

As a result the Operator address can be the same the testcorevalcons1star39u2c7jc6hk05vc9h2ls8uspz2yq7fdvhm is different.

[dzmitryhil]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)

---

database/schema/03-staking.sql line 29 at r2 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

so bdjuno doesn't have migrations define ?

How Artem Devops will apply this change ?

He has already applied it manually, should I keep it as is and add additional file to remove the constraint?

[ysv]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil, @keyleu, @miladz68, and @wojtek-coreum)

---

database/schema/03-staking.sql line 29 at r2 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

He has already applied it manually, should I keep it as is and add additional file to remove the constraint?

IMO - yes

• first of all we should never modify it manually because this will cause inconsistency between different envs. Which I guess we already have
• All changes to DB should be done via migrations & committed to repo. If state of DBs could potentially be different within different envs migration should unify it. So here ideally we should add migration which remove indices if they exist

I was always following this rules and it worked really well everywhere.

---

database/schema/03-staking.sql line 29 at r2 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Actually not.

Based on the history we can see that the delegator created 2 validators: https://explorer.testnet-1.coreum.dev/coreum/accounts/testcore1w3lry34l3j8p94ktvzm5smp45q9mqm206ewvj8

As a result the Operator address can be the same the testcorevalcons1star39u2c7jc6hk05vc9h2ls8uspz2yq7fdvhm is different.

yes, that was kinda surprising but looks valid

[wojtek-coreum]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dzmitryhil, @keyleu, and @miladz68)

---

database/schema/03-staking.sql line 29 at r2 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

yes, that was kinda surprising but looks valid

I see, they only ensure that ValidatorAddress is unique

[dzmitryhil]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @keyleu, @miladz68, and @ysv)

---

database/schema/03-staking.sql line 29 at r2 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

IMO - yes

• first of all we should never modify it manually because this will cause inconsistency between different envs. Which I guess we already have
• All changes to DB should be done via migrations & committed to repo. If state of DBs could potentially be different within different envs migration should unify it. So here ideally we should add migration which remove indices if they exist

I was always following this rules and it worked really well everywhere.

It makes sense only if you use migration tools for it, but if the source is pure SQL, that won't work that way. The repo design is done to execute all scripts and define the final state, so the migration is out of the design. So even if I add it as a separate file it won't solve the migration problem, just simplify the execution.

---

database/schema/03-staking.sql line 29 at r2 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

I see, they only ensure that ValidatorAddress is unique

Yes.

[ysv]

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dzmitryhil, @keyleu, and @miladz68)

---

database/schema/03-staking.sql line 29 at r3 (raw file):

(
    consensus_address     TEXT   NOT NULL UNIQUE PRIMARY KEY REFERENCES validator (consensus_address),
    --  TODO check that removal of operator_address UNIQUE modifier doesn't break new migration

so how will Artem Devops apply this change ?

[dzmitryhil]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @keyleu, @miladz68, and @ysv)

---

database/schema/03-staking.sql line 29 at r3 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

so how will Artem Devops apply this change ?

Yes.

[wojtek-coreum]

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @keyleu, @miladz68, and @ysv)

[ysv]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @keyleu and @miladz68)