Skip to content

Commit

Permalink
Cleanup code and changelog (#6251)
Browse files Browse the repository at this point in the history
## Motivation

This updates the changelog after the release of `v1.6.6-hotfix1` and fixes a few typos in code.
  • Loading branch information
fasmat committed Aug 13, 2024
1 parent 3aae270 commit d372ea3
Show file tree
Hide file tree
Showing 9 changed files with 138 additions and 130 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ such and the node will continue to scan new ATXs for their validity.

### Improvements

## Release v1.6.6-hotfix1

### Improvements

* [#6248](https://github.com/spacemeshos/go-spacemesh/pull/6248) Fixed node not being able to handle more than 6.55M
ATXs per epoch.

## Release v1.6.6

### Improvements
Expand Down
103 changes: 52 additions & 51 deletions activation/handler_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import (
"github.com/spacemeshos/go-spacemesh/system"
)

var errAtxNotV2 = errors.New("ATX is not V2")

type nipostValidatorV2 interface {
IsVerifyingFullPost() bool
VRFNonceV2(smesherID types.NodeID, commitment types.ATXID, vrfNonce uint64, numUnits uint32) error
Expand Down Expand Up @@ -666,96 +668,101 @@ func (h *HandlerV2) syntacticallyValidateDeps(
return &result, nil
}

func (h *HandlerV2) checkMalicious(ctx context.Context, tx *sql.Tx, atx *activationTx) error {
func (h *HandlerV2) checkMalicious(ctx context.Context, tx *sql.Tx, atx *activationTx) (bool, error) {
malicious, err := identities.IsMalicious(tx, atx.SmesherID)
if err != nil {
return fmt.Errorf("checking if node is malicious: %w", err)
return malicious, fmt.Errorf("checking if node is malicious: %w", err)
}
if malicious {
return nil
return true, nil
}

malicious, err = h.checkDoubleMarry(ctx, tx, atx)
if err != nil {
return fmt.Errorf("checking double marry: %w", err)
return malicious, fmt.Errorf("checking double marry: %w", err)
}
if malicious {
return nil
return true, nil
}

malicious, err = h.checkDoublePost(ctx, tx, atx)
if err != nil {
return fmt.Errorf("checking double post: %w", err)
return malicious, fmt.Errorf("checking double post: %w", err)
}
if malicious {
return nil
return true, nil
}

malicious, err = h.checkDoubleMerge(ctx, tx, atx)
if err != nil {
return fmt.Errorf("checking double merge: %w", err)
return malicious, fmt.Errorf("checking double merge: %w", err)
}
if malicious {
return nil
return true, nil
}

malicious, err = h.checkPrevAtx(ctx, tx, atx)
if err != nil {
return fmt.Errorf("checking previous ATX: %w", err)
}
if malicious {
return nil
return malicious, fmt.Errorf("checking previous ATX: %w", err)
}

// TODO(mafa): contextual validation:
// 1. check double-publish = ID contributed post to two ATXs in the same epoch
// 2. check previous ATX
// 3 ID already married (same node ID in multiple marriage certificates)
// 4. two ATXs referencing the same marriage certificate in the same epoch
return nil
return malicious, err
}

func (h *HandlerV2) fetchWireAtx(ctx context.Context, tx *sql.Tx, id types.ATXID) (*wire.ActivationTxV2, error) {
var blob sql.Blob
v, err := atxs.LoadBlob(ctx, tx, id.Bytes(), &blob)
if err != nil {
return nil, fmt.Errorf("get atx blob %s: %w", id.ShortString(), err)
}
if v != types.AtxV2 {
return nil, errAtxNotV2
}
atx := &wire.ActivationTxV2{}
codec.MustDecode(blob.Bytes, atx)
return atx, nil
}

func (h *HandlerV2) checkDoubleMarry(ctx context.Context, tx *sql.Tx, atx *activationTx) (bool, error) {
for _, m := range atx.marriages {
mATX, err := identities.MarriageATX(tx, m.id)
mATXID, err := identities.MarriageATX(tx, m.id)
if err != nil {
return false, fmt.Errorf("checking if ID is married: %w", err)
}
if mATX != atx.ID() {
var blob sql.Blob
v, err := atxs.LoadBlob(ctx, tx, mATX.Bytes(), &blob)
if err != nil {
return true, fmt.Errorf("creating double marry proof: %w", err)
}
if v != types.AtxV2 {
h.logger.Fatal("Failed to create double marry malfeasance proof: ATX is not v2",
zap.Stringer("atx_id", mATX),
)
}
var otherAtx wire.ActivationTxV2
codec.MustDecode(blob.Bytes, &otherAtx)
if mATXID == atx.ID() {
continue
}

proof, err := wire.NewDoubleMarryProof(tx, atx.ActivationTxV2, &otherAtx, m.id)
if err != nil {
return true, fmt.Errorf("creating double marry proof: %w", err)
}
return true, h.malPublisher.Publish(ctx, m.id, proof)
otherAtx, err := h.fetchWireAtx(ctx, tx, mATXID)
switch {
case errors.Is(err, errAtxNotV2):
h.logger.Fatal("Failed to create double marry malfeasance proof: ATX is not v2",
zap.Stringer("atx_id", mATXID),
)
case err != nil:
return false, fmt.Errorf("fetching other ATX: %w", err)
}

proof, err := wire.NewDoubleMarryProof(tx, atx.ActivationTxV2, otherAtx, m.id)
if err != nil {
return true, fmt.Errorf("creating double marry proof: %w", err)
}
return true, h.malPublisher.Publish(ctx, m.id, proof)
}
return false, nil
}

func (h *HandlerV2) checkDoublePost(ctx context.Context, tx *sql.Tx, atx *activationTx) (bool, error) {
for id := range atx.ids {
atxids, err := atxs.FindDoublePublish(tx, id, atx.PublishEpoch)
atxIDs, err := atxs.FindDoublePublish(tx, id, atx.PublishEpoch)
switch {
case errors.Is(err, sql.ErrNotFound):
continue
case err != nil:
return false, fmt.Errorf("searching for double publish: %w", err)
}
otherAtxId := slices.IndexFunc(atxids, func(other types.ATXID) bool { return other != atx.ID() })
otherAtx := atxids[otherAtxId]
otherAtxId := slices.IndexFunc(atxIDs, func(other types.ATXID) bool { return other != atx.ID() })
otherAtx := atxIDs[otherAtxId]
h.logger.Debug(
"found ID that has already contributed its PoST in this epoch",
zap.Stringer("node_id", id),
Expand Down Expand Up @@ -866,22 +873,16 @@ func (h *HandlerV2) storeAtx(ctx context.Context, atx *types.ActivationTx, watx

atxs.AtxAdded(h.cdb, atx)

var malicious bool
malicious := false
err := h.cdb.WithTx(ctx, func(tx *sql.Tx) error {
// malfeasance check happens after storing the ATX because storing updates the marriage set
// that is needed for the malfeasance proof
// TODO(mafa): don't store own ATX if it would mark the node as malicious
// this probably needs to be done by validating and storing own ATXs eagerly and skipping validation in
// the gossip handler (not sync!)
err := h.checkMalicious(ctx, tx, watx)
if err != nil {
return fmt.Errorf("check malicious: %w", err)
}
malicious, err = identities.IsMalicious(tx, watx.SmesherID)
if err != nil {
return fmt.Errorf("checking if identity is malicious: %w", err)
}
return nil
var err error
malicious, err = h.checkMalicious(ctx, tx, watx)
return err
})
if err != nil {
return fmt.Errorf("check malicious: %w", err)
Expand Down
16 changes: 8 additions & 8 deletions activation/wire/malfeasance_double_marry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func Test_DoubleMarryProof(t *testing.T) {
require.NoError(t, err)

t.Run("valid", func(t *testing.T) {
db := sql.InMemory()
db := sql.InMemoryTest(t)
otherAtx := &types.ActivationTx{}
otherAtx.SetID(types.RandomATXID())
otherAtx.SmesherID = otherSig.NodeID()
Expand Down Expand Up @@ -50,7 +50,7 @@ func Test_DoubleMarryProof(t *testing.T) {
})

t.Run("does not contain same certificate owner", func(t *testing.T) {
db := sql.InMemory()
db := sql.InMemoryTest(t)

atx1 := newActivationTxV2(
withMarriageCertificate(sig, types.EmptyATXID, sig.NodeID()),
Expand Down Expand Up @@ -79,7 +79,7 @@ func Test_DoubleMarryProof(t *testing.T) {
atx1 := newActivationTxV2()
atx1.Sign(sig)

db := sql.InMemory()
db := sql.InMemoryTest(t)
proof, err := NewDoubleMarryProof(db, atx1, atx1, sig.NodeID())
require.ErrorContains(t, err, "ATXs have the same ID")
require.Nil(t, proof)
Expand All @@ -103,7 +103,7 @@ func Test_DoubleMarryProof(t *testing.T) {
})

t.Run("invalid marriage proof", func(t *testing.T) {
db := sql.InMemory()
db := sql.InMemoryTest(t)
otherAtx := &types.ActivationTx{}
otherAtx.SetID(types.RandomATXID())
otherAtx.SmesherID = otherSig.NodeID()
Expand Down Expand Up @@ -150,7 +150,7 @@ func Test_DoubleMarryProof(t *testing.T) {
})

t.Run("invalid certificate proof", func(t *testing.T) {
db := sql.InMemory()
db := sql.InMemoryTest(t)
otherAtx := &types.ActivationTx{}
otherAtx.SetID(types.RandomATXID())
otherAtx.SmesherID = otherSig.NodeID()
Expand Down Expand Up @@ -197,7 +197,7 @@ func Test_DoubleMarryProof(t *testing.T) {
})

t.Run("invalid atx signature", func(t *testing.T) {
db := sql.InMemory()
db := sql.InMemoryTest(t)
otherAtx := &types.ActivationTx{}
otherAtx.SetID(types.RandomATXID())
otherAtx.SmesherID = otherSig.NodeID()
Expand Down Expand Up @@ -233,7 +233,7 @@ func Test_DoubleMarryProof(t *testing.T) {
})

t.Run("invalid certificate signature", func(t *testing.T) {
db := sql.InMemory()
db := sql.InMemoryTest(t)
otherAtx := &types.ActivationTx{}
otherAtx.SetID(types.RandomATXID())
otherAtx.SmesherID = otherSig.NodeID()
Expand Down Expand Up @@ -269,7 +269,7 @@ func Test_DoubleMarryProof(t *testing.T) {
})

t.Run("unknown reference ATX", func(t *testing.T) {
db := sql.InMemory()
db := sql.InMemoryTest(t)

atx1 := newActivationTxV2(
withMarriageCertificate(sig, types.EmptyATXID, sig.NodeID()),
Expand Down
32 changes: 16 additions & 16 deletions hare3/hare.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,9 @@ type WeakCoinOutput struct {

type Opt func(*Hare)

func WithWallclock(clock clockwork.Clock) Opt {
func WithWallClock(clock clockwork.Clock) Opt {
return func(hr *Hare) {
hr.wallclock = clock
hr.wallClock = clock
}
}

Expand Down Expand Up @@ -163,15 +163,15 @@ func WithResultsChan(c chan hare4.ConsensusOutput) Opt {
}
}

type nodeclock interface {
type nodeClock interface {
AwaitLayer(types.LayerID) <-chan struct{}
CurrentLayer() types.LayerID
LayerToTime(types.LayerID) time.Time
}

func New(
nodeclock nodeclock,
pubsub pubsub.PublishSubsciber,
nodeClock nodeClock,
pubsub pubsub.PublishSubscriber,
db *sql.Database,
atxsdata *atxsdata.Data,
proposals *store.Store,
Expand All @@ -192,9 +192,9 @@ func New(

config: DefaultConfig(),
log: zap.NewNop(),
wallclock: clockwork.NewRealClock(),
wallClock: clockwork.NewRealClock(),

nodeclock: nodeclock,
nodeClock: nodeClock,
pubsub: pubsub,
db: db,
atxsdata: atxsdata,
Expand Down Expand Up @@ -229,11 +229,11 @@ type Hare struct {
// options
config Config
log *zap.Logger
wallclock clockwork.Clock
wallClock clockwork.Clock

// dependencies
nodeclock nodeclock
pubsub pubsub.PublishSubsciber
nodeClock nodeClock
pubsub pubsub.PublishSubscriber
db *sql.Database
atxsdata *atxsdata.Data
proposals *store.Store
Expand Down Expand Up @@ -261,7 +261,7 @@ func (h *Hare) Coins() <-chan hare4.WeakCoinOutput {

func (h *Hare) Start() {
h.pubsub.Register(h.config.ProtocolName, h.Handler, pubsub.WithValidatorInline(true))
current := h.nodeclock.CurrentLayer() + 1
current := h.nodeClock.CurrentLayer() + 1
enabled := max(current, h.config.EnableLayer, types.GetEffectiveGenesis()+1)
disabled := types.LayerID(math.MaxUint32)
if h.config.DisableLayer > 0 {
Expand All @@ -275,7 +275,7 @@ func (h *Hare) Start() {
h.eg.Go(func() error {
for next := enabled; next < disabled; next++ {
select {
case <-h.nodeclock.AwaitLayer(next):
case <-h.nodeClock.AwaitLayer(next):
h.log.Debug("notified", zap.Uint32("lid", next.Uint32()))
h.onLayer(next)
case <-h.ctx.Done():
Expand Down Expand Up @@ -349,7 +349,7 @@ func (h *Hare) Handler(ctx context.Context, peer p2p.Peer, buf []byte) error {
droppedMessages.Inc()
return errors.New("dropped by graded gossip")
}
expected := h.nodeclock.LayerToTime(msg.Layer).Add(h.config.roundStart(msg.IterRound))
expected := h.nodeClock.LayerToTime(msg.Layer).Add(h.config.roundStart(msg.IterRound))
metrics.ReportMessageLatency(h.config.ProtocolName, msg.Round.String(), time.Since(expected))
return nil
}
Expand Down Expand Up @@ -426,12 +426,12 @@ func (h *Hare) run(session *session) error {
h.tracer.OnActive(session.vrfs)
activeLatency.Observe(time.Since(start).Seconds())

walltime := h.nodeclock.LayerToTime(session.lid).Add(h.config.PreroundDelay)
walltime := h.nodeClock.LayerToTime(session.lid).Add(h.config.PreroundDelay)
if active {
h.log.Debug("active in preround. waiting for preround delay", zap.Uint32("lid", session.lid.Uint32()))
// initial set is not needed if node is not active in preround
select {
case <-h.wallclock.After(walltime.Sub(h.wallclock.Now())):
case <-h.wallClock.After(walltime.Sub(h.wallClock.Now())):
case <-h.ctx.Done():
return h.ctx.Err()
}
Expand Down Expand Up @@ -459,7 +459,7 @@ func (h *Hare) run(session *session) error {
activeLatency.Observe(time.Since(start).Seconds())

select {
case <-h.wallclock.After(walltime.Sub(h.wallclock.Now())):
case <-h.wallClock.After(walltime.Sub(h.wallClock.Now())):
h.log.Debug("execute round",
zap.Uint32("lid", session.lid.Uint32()),
zap.Uint8("iter", session.proto.Iter), zap.Stringer("round", session.proto.Round),
Expand Down
Loading

0 comments on commit d372ea3

Please sign in to comment.