Skip to content

Commit

Permalink
chore!: remove unnecessary type method from QGB attestations (#1825)
Browse files Browse the repository at this point in the history
<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview

<!-- 
Please provide an explanation of the PR, including the appropriate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue. 
-->

Closes #1804

## Checklist

<!-- 
Please complete the checklist to ensure that the PR is ready to be
reviewed.

IMPORTANT:
PRs should be left in Draft until the below checklist is completed.
-->

- [ ] New and updated code has appropriate documentation
- [ ] New and updated code has new and/or updated testing
- [ ] Required CI checks are passing
- [ ] Visual proof for any user facing features like CLI or
documentation updates
- [ ] Linked issues closed with keywords
  • Loading branch information
rach-id authored May 24, 2023
1 parent a9a0d36 commit 456e6fd
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 47 deletions.
8 changes: 2 additions & 6 deletions x/qgb/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ func TestFirstAttestationIsValset(t *testing.T) {
require.Equal(t, uint64(1), attestation.GetNonce())

// get the valset
require.Equal(t, types.ValsetRequestType, attestation.Type())
vs, ok := attestation.(*types.Valset)
assert.True(t, ok)
assert.NotNil(t, vs)
Expand Down Expand Up @@ -133,7 +132,6 @@ func TestSetDataCommitment(t *testing.T) {
require.Equal(t, uint64(1), attestation.GetNonce())

// get the data commitment
require.Equal(t, types.DataCommitmentRequestType, attestation.Type())
actualDC, ok := attestation.(*types.DataCommitment)
assert.True(t, ok)
assert.NotNil(t, actualDC)
Expand Down Expand Up @@ -275,7 +273,6 @@ func TestDataCommitmentRange(t *testing.T) {
require.NoError(t, err)
require.True(t, found)

assert.Equal(t, types.DataCommitmentRequestType, att1.Type())
dc1, ok := att1.(*types.DataCommitment)
require.True(t, ok)
assert.Equal(t, newHeight, int64(dc1.EndBlock))
Expand All @@ -290,7 +287,6 @@ func TestDataCommitmentRange(t *testing.T) {
require.NoError(t, err)
require.True(t, found)

assert.Equal(t, types.DataCommitmentRequestType, att2.Type())
dc2, ok := att2.(*types.DataCommitment)
require.True(t, ok)
assert.Equal(t, newHeight, int64(dc2.EndBlock))
Expand Down Expand Up @@ -572,14 +568,14 @@ func TestPruning(t *testing.T) {
at, found, err := qgbKeeper.GetAttestationByNonce(ctx, nonce)
assert.NoError(t, err)
assert.True(t, found)
assert.Equal(t, types.DataCommitmentRequestType, at.Type())
_, ok := at.(*types.DataCommitment)
assert.True(t, ok)
}

// check that we still can get a valset even after pruning all of them
vs, err := qgbKeeper.GetLatestValset(ctx)
assert.NoError(t, err)
assert.NotNil(t, vs)
assert.Equal(t, types.ValsetRequestType, vs.Type())

// continue running the chain for a few more blocks to be sure no inconsistency happens
// after pruning
Expand Down
36 changes: 4 additions & 32 deletions x/qgb/keeper/keeper_valset.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,8 @@ func (k Keeper) GetLatestValset(ctx sdk.Context) (*types.Valset, error) {
fmt.Sprintf("stumbled upon nil attestation for nonce %d", i),
))
}
if at.Type() == types.ValsetRequestType {
valset, ok := at.(*types.Valset)
if !ok {
return nil, errors.Wrap(types.ErrAttestationNotValsetRequest, "couldn't cast attestation to valset")
}
valset, ok := at.(*types.Valset)
if ok {
return valset, nil
}
}
Expand Down Expand Up @@ -179,11 +176,8 @@ func (k Keeper) GetLatestValsetBeforeNonce(ctx sdk.Context, nonce uint64) (*type
fmt.Sprintf("nonce=%d", i),
)
}
if at.Type() == types.ValsetRequestType {
valset, ok := at.(*types.Valset)
if !ok {
return nil, errors.Wrap(types.ErrAttestationNotValsetRequest, "couldn't cast attestation to valset")
}
valset, ok := at.(*types.Valset)
if ok {
return valset, nil
}
}
Expand All @@ -192,25 +186,3 @@ func (k Keeper) GetLatestValsetBeforeNonce(ctx sdk.Context, nonce uint64) (*type
fmt.Sprintf("couldn't find valset before nonce %d", nonce),
)
}

// TODO add query for this method and make the orchestrator Querier use it.
// GetValsetByNonce returns the stored valset associated with the provided nonce.
// Returns (nil, false, nil) if not found.
func (k Keeper) GetValsetByNonce(ctx sdk.Context, nonce uint64) (*types.Valset, bool, error) {
at, found, err := k.GetAttestationByNonce(ctx, nonce)
if err != nil {
return nil, false, err
}
if !found {
return nil, false, nil
}
if at.Type() != types.ValsetRequestType {
return nil, false, errors.Wrap(types.ErrAttestationNotValsetRequest, "attestation is not a valset request")
}

valset, ok := at.(*types.Valset)
if !ok {
return nil, false, errors.Wrap(types.ErrAttestationNotValsetRequest, "couldn't cast attestation to valset")
}
return valset, true, nil
}
1 change: 0 additions & 1 deletion x/qgb/types/attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const (
type AttestationRequestI interface {
proto.Message
codec.ProtoMarshaler
Type() AttestationType
GetNonce() uint64
BlockTime() time.Time
}
4 changes: 0 additions & 4 deletions x/qgb/types/data_commitment.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ func NewDataCommitment(
}
}

func (m *DataCommitment) Type() AttestationType {
return DataCommitmentRequestType
}

func (m *DataCommitment) BlockTime() time.Time {
return m.Time
}
4 changes: 0 additions & 4 deletions x/qgb/types/valset.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ func (v *Valset) TwoThirdsThreshold() uint64 {
return 2 * oneThird
}

func (v *Valset) Type() AttestationType {
return ValsetRequestType
}

func (v *Valset) BlockTime() time.Time {
return v.Time
}

0 comments on commit 456e6fd

Please sign in to comment.