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.

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

fix: query db instead for validator tombstoned status #411

Closed
wants to merge 6 commits into from
Closed

fix: query db instead for validator tombstoned status #411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
10f1935
query db instead for validator tombstone status
huichiaotsou May 26, 2022
310e12e
Merge branch 'cosmos/v0.44.x' into aaron/replace_tombstone_query
huichiaotsou May 26, 2022
b33db57
add change log
huichiaotsou May 26, 2022
5e08283
Update database/slashing.go
MonikaCat May 26, 2022
3dce5c3
update changelog
huichiaotsou Jun 1, 2022
9d21889
Merge branch 'cosmos/v0.44.x' into aaron/replace_tombstone_query
huichiaotsou Jun 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
### Dependencies
- ([\#412](https://github.com/forbole/bdjuno/pull/412)) Updated Juno to `v3.2.1`

#### Staking module
- ([\#411](https://github.com/forbole/bdjuno/pull/411)) Get validator tombstoned status from the database instead of querying to the node

## Version v3.1.0
### Dependencies
- Updated Juno to `v3.2.0`
Expand Down
2 changes: 1 addition & 1 deletion cmd/parse/staking/validators.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func validatorsCmd(parseConfig *parsecmdtypes.Config) *cobra.Command {
db := database.Cast(parseCtx.Database)

// Build the staking module
stakingModule := staking.NewModule(sources.StakingSource, nil, parseCtx.EncodingConfig.Marshaler, db)
stakingModule := staking.NewModule(sources.StakingSource, parseCtx.EncodingConfig.Marshaler, db)

// Get latest height
height, err := parseCtx.Node.LatestHeight()
Expand Down
17 changes: 17 additions & 0 deletions database/slashing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,20 @@ WHERE slashing_params.height <= excluded.height`

return nil
}

// GetValidatorTombstonedStatus gets the validator latest tombstoned status from the database
func (db *Db) GetValidatorTombstonedStatus(valConsAddress string) (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.

I only have question, which I don't remember the answer to: if we're querying this value, when are we updating it? What would happen is a validator is jailed or tombstoned in a new block? Is the value updated somewhere else or what?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hey Ricky it's just wingman requested to have the tombstone status in the validator_status(x/staking) table while we used to save it only in validator_signing_info(x/slashing). So:

  • the tombstone status is updated with the slashing module
  • in the staking module the status is grabbed from validator_signing_info and stored again in validator_status

Copy link
Contributor

Choose a reason for hiding this comment

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

@ryuash Can we have some more context on why this is needed/required/preferable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise we just get the tombstone status from the validator_signing_info table, gql query be something like this

query MyQuery {
  validator_status {
    height
    jailed
    status
    validator_address
    validator {
      validator_signing_infos {
        tombstoned
      }
    }
  }
}

in this case I will remove the tomstoned column from validator_status table.

would this be reasonable for you @yayay927 ?

Copy link

Choose a reason for hiding this comment

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

I remember this was requested months back. I no longer remember the purpose tbh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

then I think we can remove this part of codes because it keeps causing "transport is closing" error while calling the node :-/ we should then alternatively JOIN the validator_signing_info table for tombstone status.

Copy link

@ryuash ryuash Jun 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

OH SORRY I REMEMBER NOW. Yes this was requested during the first wave of tombstones validators as we had no way of seeing who was tombstoned in bdjuno at the time.

It's currently being used in the frontend so a migration would be needed if you're making any changes.

Would moving it to a different table be better or? I figured bdjuno was listening for the tombstone event to update this but I could be wrong

Copy link
Contributor Author

@huichiaotsou huichiaotsou Jun 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Cool so front end will get the tombstone from validator signing info first and then we can remove those codes from BDjuno safely :D

Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly if BD is fine as it is right now, I would avoid updating anything

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, but we actually get a lot of errors like this while running staking module

error="error while getting validator signing info: 
rpc error: code = Unavailable desc = transport is closing" height=621512 module=staking

saving tombstone twice is weird no? when we can just join the tables

stmt := `SELECT tombstoned FROM validator_signing_info WHERE validator_address = $1`

var rows []bool
err := db.Sqlx.Select(&rows, stmt, valConsAddress)
if err != nil {
return false, fmt.Errorf("error while gettting validator tombstoned status: %s", err)
}

if len(rows) == 0 {
return false, fmt.Errorf("validator tombstoned status is empty")
}

return rows[0], nil
}
2 changes: 1 addition & 1 deletion modules/registrar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (r *Registrar) BuildModules(ctx registrar.Context) jmodules.Modules {
feegrantModule := feegrant.NewModule(cdc, db)
mintModule := mint.NewModule(sources.MintSource, cdc, db)
slashingModule := slashing.NewModule(sources.SlashingSource, cdc, db)
stakingModule := staking.NewModule(sources.StakingSource, slashingModule, cdc, db)
stakingModule := staking.NewModule(sources.StakingSource, cdc, db)
govModule := gov.NewModule(sources.GovSource, authModule, distrModule, mintModule, slashingModule, stakingModule, cdc, db)

return []jmodules.Module{
Expand Down
11 changes: 0 additions & 11 deletions modules/staking/expected_modules.go
View file
Open in desktop

This file was deleted.

16 changes: 7 additions & 9 deletions modules/staking/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,20 @@ var (

// Module represents the x/staking module
type Module struct {
cdc codec.Codec
db *database.Db
source stakingsource.Source
slashingModule SlashingModule
cdc codec.Codec
db *database.Db
source stakingsource.Source
}

// NewModule returns a new Module instance
func NewModule(
source stakingsource.Source, slashingModule SlashingModule,
source stakingsource.Source,
cdc codec.Codec, db *database.Db,
) *Module {
return &Module{
cdc: cdc,
db: db,
source: source,
slashingModule: slashingModule,
cdc: cdc,
db: db,
source: source,
}
}

Expand Down
6 changes: 3 additions & 3 deletions modules/staking/utils_validators.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,17 +176,17 @@ func (m *Module) GetValidatorsStatuses(height int64, validators []stakingtypes.V
return nil, fmt.Errorf("error while getting validator consensus public key: %s", err)
}

valSigningInfo, err := m.slashingModule.GetSigningInfo(height, consAddr)
valTombstonedStatus, err := m.db.GetValidatorTombstonedStatus(consAddr.String())
if err != nil && !strings.Contains(err.Error(), codes.NotFound.String()) {
return nil, fmt.Errorf("error while getting validator signing info: %s", err)
return nil, fmt.Errorf("error while getting validator tombstoned status: %s", err)
}

statuses[index] = types.NewValidatorStatus(
consAddr.String(),
consPubKey.String(),
int(validator.GetStatus()),
validator.IsJailed(),
valSigningInfo.Tombstoned,
valTombstonedStatus,
height,
)
}
Expand Down