From 213a58503544b24684acdd29fbffd6376876b996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20R=C3=B3=C5=BCa=C5=84ski?= Date: Mon, 22 Apr 2024 08:49:48 +0000 Subject: [PATCH] Avoid reencoding ATX blob (#5868) ## Motivation --- activation/activation.go | 13 +++++++--- activation/handler.go | 5 ++-- activation/wire/wire_v1.go | 11 +++++++-- checkpoint/recovery_test.go | 4 ++-- common/types/activation.go | 10 ++++++++ malfeasance/wire/malfeasance.go | 6 ++--- sql/atxs/atxs.go | 24 ++++++++++--------- sql/atxs/atxs_test.go | 8 ++++--- .../state/0018_atx_blob_version.sql | 4 ++++ 9 files changed, 58 insertions(+), 27 deletions(-) create mode 100644 sql/migrations/state/0018_atx_blob_version.sql diff --git a/activation/activation.go b/activation/activation.go index 8d3a2da602..267d92421a 100644 --- a/activation/activation.go +++ b/activation/activation.go @@ -760,11 +760,18 @@ func (b *Builder) Regossip(ctx context.Context, nodeID types.NodeID) error { } // SignAndFinalizeAtx signs the atx with specified signer and calculates the ID of the ATX. +// DO NOT USE for new code. This function is deprecated and will be removed. +// The proper way to create an ATX in tests is to use the specific wire type and sign it. func SignAndFinalizeAtx(signer *signing.EdSigner, atx *types.ActivationTx) error { - // FIXME - there is no need to sign types.ActivationTX (only ActivationTxVx) wireAtx := wire.ActivationTxToWireV1(atx) - atx.Signature = signer.Sign(signing.ATX, wireAtx.SignedBytes()) - atx.SmesherID = signer.NodeID() + wireAtx.Signature = signer.Sign(signing.ATX, wireAtx.SignedBytes()) + wireAtx.SmesherID = signer.NodeID() + atx.AtxBlob = types.AtxBlob{ + Version: types.AtxV1, + Blob: codec.MustEncode(wireAtx), + } + atx.Signature = wireAtx.Signature + atx.SmesherID = wireAtx.SmesherID atx.SetID(types.ATXID(wireAtx.HashInnerBytes())) return nil } diff --git a/activation/handler.go b/activation/handler.go index 3d50055260..8818a43606 100644 --- a/activation/handler.go +++ b/activation/handler.go @@ -511,7 +511,7 @@ func (h *Handler) handleAtx( h.inProgressMu.Unlock() h.log.WithContext(ctx).With().Info("handling incoming atx", id, log.Int("size", len(msg))) - proof, err := h.processATX(ctx, peer, atx, receivedTime) + proof, err := h.processATX(ctx, peer, atx, msg, receivedTime) h.inProgressMu.Lock() defer h.inProgressMu.Unlock() for _, ch := range h.inProgress[id] { @@ -526,6 +526,7 @@ func (h *Handler) processATX( ctx context.Context, peer p2p.Peer, watx wire.ActivationTxV1, + blob []byte, received time.Time, ) (*mwire.MalfeasanceProof, error) { if !h.edVerifier.Verify(signing.ATX, watx.SmesherID, watx.SignedBytes(), watx.Signature) { @@ -575,7 +576,7 @@ func (h *Handler) processATX( baseTickHeight = posAtx.TickHeight() } - atx := wire.ActivationTxFromWireV1(&watx) + atx := wire.ActivationTxFromWireV1(&watx, blob...) if h.nipostValidator.IsVerifyingFullPost() { atx.SetValidity(types.Valid) } diff --git a/activation/wire/wire_v1.go b/activation/wire/wire_v1.go index eb86adb62a..907b2958b2 100644 --- a/activation/wire/wire_v1.go +++ b/activation/wire/wire_v1.go @@ -173,10 +173,10 @@ func ActivationTxFromBytes(data []byte) (*types.ActivationTx, error) { return nil, fmt.Errorf("decoding ATX: %w", err) } - return ActivationTxFromWireV1(&wireAtx), nil + return ActivationTxFromWireV1(&wireAtx, data...), nil } -func ActivationTxFromWireV1(atx *ActivationTxV1) *types.ActivationTx { +func ActivationTxFromWireV1(atx *ActivationTxV1, blob ...byte) *types.ActivationTx { result := &types.ActivationTx{ InnerActivationTx: types.InnerActivationTx{ NIPostChallenge: types.NIPostChallenge{ @@ -195,6 +195,13 @@ func ActivationTxFromWireV1(atx *ActivationTxV1) *types.ActivationTx { }, SmesherID: atx.SmesherID, Signature: atx.Signature, + AtxBlob: types.AtxBlob{ + Version: types.AtxV1, + Blob: blob, + }, + } + if len(blob) == 0 { + result.AtxBlob.Blob = codec.MustEncode(atx) } result.SetID(types.ATXID(atx.HashInnerBytes())) diff --git a/checkpoint/recovery_test.go b/checkpoint/recovery_test.go index 8707e10c5f..76631a1a22 100644 --- a/checkpoint/recovery_test.go +++ b/checkpoint/recovery_test.go @@ -256,7 +256,7 @@ func validateAndPreserveData( for _, dep := range deps { var atx wire.ActivationTxV1 require.NoError(tb, codec.Decode(dep.Blob, &atx)) - vatx := wire.ActivationTxFromWireV1(&atx) + vatx := wire.ActivationTxFromWireV1(&atx, dep.Blob...) mclock.EXPECT().CurrentLayer().Return(vatx.PublishEpoch.FirstLayer()) mfetch.EXPECT().RegisterPeerHashes(gomock.Any(), gomock.Any()) mfetch.EXPECT().GetPoetProof(gomock.Any(), gomock.Any()) @@ -795,7 +795,7 @@ func TestRecover_OwnAtxNotInCheckpoint_Preserve_DepIsGolden(t *testing.T) { // make the first one from the previous snapshot var atx wire.ActivationTxV1 require.NoError(t, codec.Decode(vAtxs[0].Blob, &atx)) - golden := wire.ActivationTxFromWireV1(&atx) + golden := wire.ActivationTxFromWireV1(&atx, vAtxs[0].Blob...) require.NoError(t, atxs.AddCheckpointed(oldDB, &atxs.CheckpointAtx{ ID: golden.ID(), Epoch: golden.PublishEpoch, diff --git a/common/types/activation.go b/common/types/activation.go index 6511e1ece1..2d4e889a4e 100644 --- a/common/types/activation.go +++ b/common/types/activation.go @@ -158,6 +158,15 @@ func (m *ATXMetadata) MarshalLogObject(encoder log.ObjectEncoder) error { return nil } +type AtxVersion uint + +const AtxV1 AtxVersion = 1 + +type AtxBlob struct { + Blob []byte + Version AtxVersion +} + // ActivationTx is a full, signed activation transaction. It includes (or references) everything a miner needs to prove // they are eligible to actively participate in the Spacemesh protocol in the next epoch. type ActivationTx struct { @@ -166,6 +175,7 @@ type ActivationTx struct { SmesherID NodeID Signature EdSignature + AtxBlob golden bool } diff --git a/malfeasance/wire/malfeasance.go b/malfeasance/wire/malfeasance.go index c5a21e8831..51e5e97980 100644 --- a/malfeasance/wire/malfeasance.go +++ b/malfeasance/wire/malfeasance.go @@ -71,8 +71,7 @@ func (mp *MalfeasanceProof) MarshalLogObject(encoder log.ObjectEncoder) error { encoder.AddString("type", "invalid post index") p, ok := mp.Proof.Data.(*InvalidPostIndexProof) if ok { - atx := wire.ActivationTxFromWireV1(&p.Atx) - encoder.AddString("atx_id", atx.ID().String()) + encoder.AddString("atx_id", p.Atx.ID().String()) encoder.AddString("smesher", p.Atx.SmesherID.String()) encoder.AddUint32("invalid index", p.InvalidIdx) } @@ -359,11 +358,10 @@ func MalfeasanceInfo(smesher types.NodeID, mp *MalfeasanceProof) string { case InvalidPostIndex: p, ok := mp.Proof.Data.(*InvalidPostIndexProof) if ok { - atx := wire.ActivationTxFromWireV1(&p.Atx) b.WriteString( fmt.Sprintf( "cause: smesher published ATX %s with invalid post index %d in epoch %d\n", - atx.ID().ShortString(), + p.Atx.ID().ShortString(), p.InvalidIdx, p.Atx.PublishEpoch, )) diff --git a/sql/atxs/atxs.go b/sql/atxs/atxs.go index e8c86e8da8..8ff95eb95a 100644 --- a/sql/atxs/atxs.go +++ b/sql/atxs/atxs.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "time" sqlite "github.com/go-llsqlite/crawshaw" @@ -40,11 +41,16 @@ func decoder(fn decoderCallback) sql.Decoder { stmt.ColumnBytes(0, id[:]) checkpointed := stmt.ColumnLen(1) == 0 if !checkpointed { + // FIXME: remove decoding blob once ActivationTx struct is trimmed from unncecessary fields. var atxV1 wire.ActivationTxV1 - if _, err := codec.DecodeFrom(stmt.ColumnReader(1), &atxV1); err != nil { + blob, err := io.ReadAll(stmt.ColumnReader(1)) + if err != nil { + return fn(nil, fmt.Errorf("read blob %w", err)) + } + if err := codec.Decode(blob, &atxV1); err != nil { return fn(nil, fmt.Errorf("decode %w", err)) } - a = *wire.ActivationTxFromWireV1(&atxV1) + a = *wire.ActivationTxFromWireV1(&atxV1, blob...) } a.SetID(id) baseTickHeight := uint64(stmt.ColumnInt64(2)) @@ -423,11 +429,6 @@ func AddMaybeNoNonce(db sql.Executor, atx *types.VerifiedActivationTx) error { } func add(db sql.Executor, atx *types.VerifiedActivationTx, nonce *types.VRFPostIndex) error { - buf, err := codec.Encode(wire.ActivationTxToWireV1(atx.ActivationTx)) - if err != nil { - return fmt.Errorf("encode: %w", err) - } - enc := func(stmt *sql.Statement) { stmt.BindBytes(1, atx.ID().Bytes()) stmt.BindInt64(2, int64(atx.PublishEpoch)) @@ -456,7 +457,7 @@ func add(db sql.Executor, atx *types.VerifiedActivationTx, nonce *types.VRFPostI } } - _, err = db.Exec(` + _, err := db.Exec(` insert into atxs (id, epoch, effective_num_units, commitment_atx, nonce, pubkey, received, base_tick_height, tick_count, sequence, coinbase, validity, prev_id) @@ -467,9 +468,10 @@ func add(db sql.Executor, atx *types.VerifiedActivationTx, nonce *types.VRFPostI enc = func(stmt *sql.Statement) { stmt.BindBytes(1, atx.ID().Bytes()) - stmt.BindBytes(2, buf) + stmt.BindBytes(2, atx.Blob) + stmt.BindInt64(3, int64(atx.Version)) } - _, err = db.Exec("insert into atx_blobs (id, atx) values (?1, ?2)", enc, nil) + _, err = db.Exec("insert into atx_blobs (id, atx, version) values (?1, ?2, ?3)", enc, nil) if err != nil { return fmt.Errorf("insert ATX blob %v: %w", atx.ID(), err) } @@ -801,7 +803,7 @@ func PoetProofRef(ctx context.Context, db sql.Executor, id types.ATXID) (types.P return types.PoetProofRef{}, fmt.Errorf("getting blob for %s: %w", id, err) } - // TODO: decide about version based on publish epoch + // TODO: decide about version based the `version` column in `atx_blobs` var atx wire.ActivationTxV1 if err := codec.Decode(blob.Bytes, &atx); err != nil { return types.PoetProofRef{}, fmt.Errorf("decoding ATX blob: %w", err) diff --git a/sql/atxs/atxs_test.go b/sql/atxs/atxs_test.go index a3f35cf9bc..2f6c191d5c 100644 --- a/sql/atxs/atxs_test.go +++ b/sql/atxs/atxs_test.go @@ -597,9 +597,11 @@ func TestLoadBlob(t *testing.T) { require.Equal(t, []int{len(blob1.Bytes)}, blobSizes) var blob2 sql.Blob - atx2, err := newAtx(sig, withPublishEpoch(1)) - nodeID := types.RandomNodeID() - atx2.NodeID = &nodeID // ensure ATXs differ in size + atx2, err := newAtx(sig, func(atx *types.ActivationTx) { + nodeID := types.RandomNodeID() + atx.NodeID = &nodeID // ensure ATXs differ in size + }) + require.NoError(t, err) require.NoError(t, atxs.Add(db, atx2)) require.NoError(t, atxs.LoadBlob(ctx, db, atx2.ID().Bytes(), &blob2)) diff --git a/sql/migrations/state/0018_atx_blob_version.sql b/sql/migrations/state/0018_atx_blob_version.sql new file mode 100644 index 0000000000..0cec04339b --- /dev/null +++ b/sql/migrations/state/0018_atx_blob_version.sql @@ -0,0 +1,4 @@ +-- Add version column to make it easier to decode the blob +-- to the right version of ATX. +ALTER TABLE atx_blobs ADD COLUMN version INTEGER; +UPDATE atx_blobs SET version = 1;