Skip to content

Commit

Permalink
chore: code cleanup (#6474)
Browse files Browse the repository at this point in the history
## Motivation

I removed some dead code from the codebase and cleaned up some other issues my linter complained about.
  • Loading branch information
fasmat committed Nov 20, 2024
1 parent 039ab4f commit 69957b2
Show file tree
Hide file tree
Showing 34 changed files with 109 additions and 322 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

See [RELEASE](./RELEASE.md) for workflow instructions.

## UNRELEASED

### Upgrade information

### Highlights

### Features

### Improvements

## v1.7.7

### Improvements
Expand Down
10 changes: 5 additions & 5 deletions activation/activation.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
)

var (
ErrNotFound = errors.New("not found")
errNotFound = errors.New("not found")
errNilVrfNonce = errors.New("nil VRF nonce")
)

Expand Down Expand Up @@ -429,7 +429,7 @@ func (b *Builder) run(ctx context.Context, sig *signing.EdSigner) {
eg.Go(func() error {
_, err := poet.Certify(ctx, sig.NodeID())
switch {
case errors.Is(err, ErrCertificatesNotSupported):
case errors.Is(err, errCertificatesNotSupported):
b.logger.Debug("not certifying (not supported in poet)",
log.ZShortStringer("smesherID", sig.NodeID()),
zap.String("poet", poet.Address()),
Expand All @@ -454,7 +454,7 @@ func (b *Builder) run(ctx context.Context, sig *signing.EdSigner) {

poetErr := &PoetSvcUnstableError{}
switch {
case errors.Is(err, ErrATXChallengeExpired):
case errors.Is(err, errATXChallengeExpired):
b.logger.Debug("retrying with new challenge after waiting for a layer")
if err := b.nipostBuilder.ResetState(sig.NodeID()); err != nil {
b.logger.Error("failed to reset nipost builder state", zap.Error(err))
Expand Down Expand Up @@ -829,7 +829,7 @@ func (b *Builder) createAtx(
// initial NIPoST challenge is not discarded; don't return ErrATXChallengeExpired
return nil, errors.New("atx publish epoch has passed during nipost construction")
}
return nil, fmt.Errorf("%w: atx publish epoch has passed during nipost construction", ErrATXChallengeExpired)
return nil, fmt.Errorf("%w: atx publish epoch has passed during nipost construction", errATXChallengeExpired)
}

switch version {
Expand Down Expand Up @@ -1086,7 +1086,7 @@ func findFullyValidHighTickAtx(
}

if found == nil {
return types.ATXID{}, ErrNotFound
return types.ATXID{}, errNotFound
}

return *found, nil
Expand Down
8 changes: 4 additions & 4 deletions activation/activation_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import (
)

var (
// ErrATXChallengeExpired is returned when atx missed its publication window and needs to be regenerated.
ErrATXChallengeExpired = errors.New("builder: atx expired")
// ErrPoetProofNotReceived is returned when no poet proof was received.
ErrPoetProofNotReceived = errors.New("builder: didn't receive any poet proof")
// errATXChallengeExpired is returned when atx missed its publication window and needs to be regenerated.
errATXChallengeExpired = errors.New("builder: atx expired")
// errPoetProofNotReceived is returned when no poet proof was received.
errPoetProofNotReceived = errors.New("builder: didn't receive any poet proof")
)

// PoetSvcUnstableError means there was a problem communicating
Expand Down
4 changes: 2 additions & 2 deletions activation/activation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ func TestBuilder_Loop_WaitsOnStaleChallenge(t *testing.T) {

tab.mnipost.EXPECT().
BuildNIPost(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Return(nil, ErrATXChallengeExpired)
Return(nil, errATXChallengeExpired)
tab.mnipost.EXPECT().ResetState(sig.NodeID()).Return(nil)

ctx, cancel := context.WithCancel(context.Background())
Expand Down Expand Up @@ -803,7 +803,7 @@ func TestBuilder_PublishActivationTx_NoPrevATX_PublishFails_InitialPost_preserve
}).AnyTimes()
tab.mnipost.EXPECT().
BuildNIPost(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Return(nil, ErrATXChallengeExpired)
Return(nil, errATXChallengeExpired)
tab.mnipost.EXPECT().ResetState(sig.NodeID()).Return(nil)
ch := make(chan struct{})
tab.mclock.EXPECT().AwaitLayer(currLayer.Add(1)).Do(func(got types.LayerID) <-chan struct{} {
Expand Down
12 changes: 6 additions & 6 deletions activation/nipost.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ func (nb *NIPostBuilder) BuildNIPost(
if poetProofDeadline.Before(now) {
return nil, fmt.Errorf(
"%w: deadline to query poet proof for pub epoch %d exceeded (deadline: %s, now: %s)",
ErrATXChallengeExpired,
errATXChallengeExpired,
postChallenge.PublishEpoch,
poetProofDeadline,
now,
Expand All @@ -276,7 +276,7 @@ func (nb *NIPostBuilder) BuildNIPost(
return nil, &PoetSvcUnstableError{msg: "getBestProof failed", source: err}
}
if poetProofRef == types.EmptyPoetProofRef {
return nil, &PoetSvcUnstableError{source: ErrPoetProofNotReceived}
return nil, &PoetSvcUnstableError{source: errPoetProofNotReceived}
}
if err := nipost.UpdatePoetProofRef(nb.localDB, signer.NodeID(), poetProofRef, membership); err != nil {
nb.logger.Warn("cannot persist poet proof ref", zap.Error(err))
Expand All @@ -295,7 +295,7 @@ func (nb *NIPostBuilder) BuildNIPost(
if publishEpochEnd.Before(now) {
return nil, fmt.Errorf(
"%w: deadline to publish ATX for pub epoch %d exceeded (deadline: %s, now: %s)",
ErrATXChallengeExpired,
errATXChallengeExpired,
postChallenge.PublishEpoch,
publishEpochEnd,
now,
Expand Down Expand Up @@ -446,7 +446,7 @@ func (nb *NIPostBuilder) submitPoetChallenges(
// no existing registration at all, drop current registration challenge
return nil, fmt.Errorf(
"%w: poet round has already started at %s (now: %s)",
ErrATXChallengeExpired,
errATXChallengeExpired,
curPoetRoundStartDeadline,
now,
)
Expand Down Expand Up @@ -499,7 +499,7 @@ func (nb *NIPostBuilder) submitPoetChallenges(

if len(existingRegistrations) == 0 {
if curPoetRoundStartDeadline.Before(time.Now()) {
return nil, ErrATXChallengeExpired
return nil, errATXChallengeExpired
}
return nil, &PoetSvcUnstableError{msg: "failed to submit challenge to any PoET", source: ctx.Err()}
}
Expand Down Expand Up @@ -616,7 +616,7 @@ func (nb *NIPostBuilder) getBestProof(
return ref, bestProof.membership, nil
}

return types.PoetProofRef{}, nil, ErrPoetProofNotReceived
return types.PoetProofRef{}, nil, errPoetProofNotReceived
}

func constructMerkleProof(challenge types.Hash32, members []types.Hash32) (*types.MerkleProof, error) {
Expand Down
12 changes: 6 additions & 6 deletions activation/nipost_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ func TestNIPSTBuilder_PoetUnstable(t *testing.T) {
challenge,
&types.NIPostChallenge{PublishEpoch: postGenesisEpoch + 2},
)
require.ErrorIs(t, err, ErrATXChallengeExpired)
require.ErrorIs(t, err, errATXChallengeExpired)
require.Nil(t, nipst)
})
t.Run("GetProof fails", func(t *testing.T) {
Expand All @@ -786,7 +786,7 @@ func TestNIPSTBuilder_PoetUnstable(t *testing.T) {

nipst, err := nb.BuildNIPost(context.Background(), sig, challenge,
&types.NIPostChallenge{PublishEpoch: postGenesisEpoch + 2})
require.ErrorIs(t, err, ErrPoetProofNotReceived)
require.ErrorIs(t, err, errPoetProofNotReceived)
require.Nil(t, nipst)
})
t.Run("Challenge is not included in proof members", func(t *testing.T) {
Expand All @@ -812,7 +812,7 @@ func TestNIPSTBuilder_PoetUnstable(t *testing.T) {

nipst, err := nb.BuildNIPost(context.Background(), sig, challenge,
&types.NIPostChallenge{PublishEpoch: postGenesisEpoch + 2})
require.ErrorIs(t, err, ErrPoetProofNotReceived)
require.ErrorIs(t, err, errPoetProofNotReceived)
require.Nil(t, nipst)
})
}
Expand Down Expand Up @@ -1152,7 +1152,7 @@ func TestNIPoSTBuilder_StaleChallenge(t *testing.T) {

nipost, err := nb.BuildNIPost(context.Background(), sig, types.RandomHash(),
&types.NIPostChallenge{PublishEpoch: currLayer.GetEpoch()})
require.ErrorIs(t, err, ErrATXChallengeExpired)
require.ErrorIs(t, err, errATXChallengeExpired)
require.ErrorContains(t, err, "poet round has already started")
require.Nil(t, nipost)
})
Expand Down Expand Up @@ -1194,7 +1194,7 @@ func TestNIPoSTBuilder_StaleChallenge(t *testing.T) {
require.NoError(t, err)

nipost, err := nb.BuildNIPost(context.Background(), sig, challengeHash, challenge)
require.ErrorIs(t, err, ErrATXChallengeExpired)
require.ErrorIs(t, err, errATXChallengeExpired)
require.ErrorContains(t, err, "poet proof for pub epoch")
require.Nil(t, nipost)
})
Expand Down Expand Up @@ -1240,7 +1240,7 @@ func TestNIPoSTBuilder_StaleChallenge(t *testing.T) {
require.NoError(t, err)

nipost, err := nb.BuildNIPost(context.Background(), sig, challengeHash, challenge)
require.ErrorIs(t, err, ErrATXChallengeExpired)
require.ErrorIs(t, err, errATXChallengeExpired)
require.ErrorContains(t, err, "deadline to publish ATX for pub epoch")
require.Nil(t, nipost)
})
Expand Down
18 changes: 9 additions & 9 deletions activation/poet.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ import (
var (
ErrInvalidRequest = errors.New("invalid request")
ErrUnauthorized = errors.New("unauthorized")
ErrCertificatesNotSupported = errors.New("poet doesn't support certificates")
ErrIncompatiblePhaseShift = errors.New("fetched poet phase_shift is incompatible with configured phase_shift")
ErrCertifierNotConfigured = errors.New("certifier service not configured")
errCertificatesNotSupported = errors.New("poet doesn't support certificates")
errIncompatiblePhaseShift = errors.New("fetched poet phase_shift is incompatible with configured phase_shift")
errCertifierNotConfigured = errors.New("certifier service not configured")
)

type PoetPowParams struct {
Expand Down Expand Up @@ -464,7 +464,7 @@ func NewPoetServiceWithClient(

err := service.verifyPhaseShiftConfiguration(context.Background())
switch {
case errors.Is(err, ErrIncompatiblePhaseShift):
case errors.Is(err, errIncompatiblePhaseShift):
logger.Fatal("failed to create poet service", zap.String("poet", client.Address()))
return nil
case err != nil:
Expand All @@ -487,7 +487,7 @@ func (c *poetService) verifyPhaseShiftConfiguration(ctx context.Context) error {
}

if info.PhaseShift != c.expectedPhaseShift {
return ErrIncompatiblePhaseShift
return errIncompatiblePhaseShift
}

return nil
Expand All @@ -508,7 +508,7 @@ func (c *poetService) authorize(
switch {
case err == nil:
return &PoetAuth{PoetCert: cert}, nil
case errors.Is(err, ErrCertificatesNotSupported):
case errors.Is(err, errCertificatesNotSupported):
logger.Debug("poet doesn't support certificates")
default:
logger.Warn("failed to certify", zap.Error(err))
Expand Down Expand Up @@ -574,7 +574,7 @@ func (c *poetService) Submit(

err := c.verifyPhaseShiftConfiguration(ctx)
switch {
case errors.Is(err, ErrIncompatiblePhaseShift):
case errors.Is(err, errIncompatiblePhaseShift):
logger.Fatal("failed to submit challenge", zap.String("poet", c.client.Address()))
return nil, err
case err != nil:
Expand Down Expand Up @@ -639,7 +639,7 @@ func (c *poetService) Proof(ctx context.Context, roundID string) (*types.PoetPro

func (c *poetService) Certify(ctx context.Context, id types.NodeID) (*certifier.PoetCert, error) {
if c.certifier == nil {
return nil, ErrCertifierNotConfigured
return nil, errCertifierNotConfigured
}

info, err := c.getInfo(ctx)
Expand All @@ -648,7 +648,7 @@ func (c *poetService) Certify(ctx context.Context, id types.NodeID) (*certifier.
}

if info.Certifier == nil {
return nil, ErrCertificatesNotSupported
return nil, errCertificatesNotSupported
}
return c.certifier.Certificate(ctx, id, info.Certifier.Url, info.Certifier.Pubkey)
}
Expand Down
2 changes: 1 addition & 1 deletion activation/poet_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ func TestPoetClient_Certify(t *testing.T) {
poet := NewPoetServiceWithClient(
nil, client, cfg, zaptest.NewLogger(t), testTickSize, WithCertifier(mCertifier))
_, err = poet.Certify(context.Background(), sig.NodeID())
require.ErrorIs(t, err, ErrCertificatesNotSupported)
require.ErrorIs(t, err, errCertificatesNotSupported)
})
}

Expand Down
2 changes: 1 addition & 1 deletion activation/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ func (mgr *PostSetupManager) findCommitmentAtx(ctx context.Context) (types.ATXID
VerifyChainOpts.WithLogger(mgr.logger),
)
switch {
case errors.Is(err, ErrNotFound):
case errors.Is(err, errNotFound):
mgr.logger.Info("using golden atx as commitment atx")
return mgr.goldenATXID, nil
case err != nil:
Expand Down
2 changes: 1 addition & 1 deletion activation/wire/malfeasance_invalid_post.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func (p InvalidPostProof) Valid(
p.NumUnits,
int(p.ValidPostIndex),
); err != nil {
return errors.New("Commitment ATX is not valid")
return errors.New("commitment ATX is not valid")
}

if err := malValidator.PostIndex(
Expand Down
2 changes: 1 addition & 1 deletion activation/wire/malfeasance_invalid_post_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func Test_InvalidPostProof(t *testing.T) {
).Return(errors.New("invalid post"))

id, err := proof.Valid(context.Background(), verifier)
require.EqualError(t, err, "invalid invalid post proof: Commitment ATX is not valid")
require.EqualError(t, err, "invalid invalid post proof: commitment ATX is not valid")
require.Equal(t, types.EmptyNodeID, id)
})

Expand Down
2 changes: 1 addition & 1 deletion cmd/bootstrapper/bootstrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func queryNetworkParams(ctx context.Context, endpoint string) (*NetworkParam, er

func main() {
if err := cmd.Execute(); err != nil {
_, _ = fmt.Fprintln(os.Stderr, err)
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}
}
2 changes: 1 addition & 1 deletion common/types/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (a Address) String() string {
// Format implements fmt.Formatter, forcing the byte slice to be formatted as is,
// without going through the stringer interface used for logging.
func (a Address) Format(s fmt.State, c rune) {
_, _ = fmt.Fprintf(s, "%"+string(c), a[:])
fmt.Fprintf(s, "%"+string(c), a[:])
}

// EncodeScale implements scale codec interface.
Expand Down
14 changes: 10 additions & 4 deletions common/types/hashes.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (h Hash20) ShortString() string {
// Format implements fmt.Formatter, forcing the byte slice to be formatted as is,
// without going through the stringer interface used for logging.
func (h Hash20) Format(s fmt.State, c rune) {
_, _ = fmt.Fprintf(s, "%"+string(c), h[:])
fmt.Fprintf(s, "%"+string(c), h[:])
}

// UnmarshalText parses a hash in hex syntax.
Expand All @@ -69,7 +69,10 @@ func (h *Hash20) UnmarshalJSON(input []byte) error {

// MarshalText returns the hex representation of h.
func (h Hash20) MarshalText() ([]byte, error) {
return util.Bytes(h[:]).MarshalText()
result := make([]byte, len(h)*2+2)
copy(result, `0x`)
hex.Encode(result[2:], h[:])
return result, nil
}

// SetBytes sets the hash to the value of b.
Expand Down Expand Up @@ -163,7 +166,7 @@ func (h Hash32) ShortString() string {
// Format implements fmt.Formatter, forcing the byte slice to be formatted as is,
// without going through the stringer interface used for logging.
func (h Hash32) Format(s fmt.State, c rune) {
_, _ = fmt.Fprintf(s, "%"+string(c), h[:])
fmt.Fprintf(s, "%"+string(c), h[:])
}

// UnmarshalText parses a hash in hex syntax.
Expand All @@ -186,7 +189,10 @@ func (h *Hash32) UnmarshalJSON(input []byte) error {

// MarshalText returns the hex representation of h.
func (h Hash32) MarshalText() ([]byte, error) {
return util.Bytes(h[:]).MarshalText()
result := make([]byte, len(h)*2+2)
copy(result, `0x`)
hex.Encode(result[2:], h[:])
return result, nil
}

// SetBytes sets the hash to the value of b.
Expand Down
2 changes: 1 addition & 1 deletion common/types/testutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func RandomTransactionID() TransactionID {
// RandomBallot generates a Ballot with random content for testing.
func RandomBallot() *Ballot {
var vrf VrfSignature
_, _ = rand.Read(vrf[:])
rand.Read(vrf[:])

return &Ballot{
InnerBallot: InnerBallot{
Expand Down
14 changes: 0 additions & 14 deletions common/util/bytes.go

This file was deleted.

Loading

0 comments on commit 69957b2

Please sign in to comment.