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

Jump to bottom

[Merged by Bors] - Verify self-generated POST proofs to catch errors early #4721

Closed
wants to merge 5 commits into from
Closed

[Merged by Bors] - Verify self-generated POST proofs to catch errors early #4721

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
65e48cf
Verify self-generated POST proofs to catch errors early
poszu Jul 18, 2023
c816d14
Fix UT
poszu Jul 18, 2023
355544d
Review fixes
poszu Jul 18, 2023
80accf0
Review fixes
poszu Jul 18, 2023
2daac62
Remove PostSetupManager::VerifyProof
poszu Jul 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 33 additions & 9 deletions activation/activation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
nipostBuilder nipostBuilder
postSetupProvider postSetupProvider
initialPost *types.Post
validator nipostValidator

// smeshingMutex protects `StartSmeshing` and `StopSmeshing` from concurrent access
smeshingMutex sync.Mutex
Expand Down Expand Up @@ -125,6 +126,12 @@
}
}

func WithValidator(v nipostValidator) BuilderOption {
return func(b *Builder) {
b.validator = v
}
}

// NewBuilder returns an atx builder that will start a routine that will attempt to create an atx upon each new layer.
func NewBuilder(
conf Config,
Expand Down Expand Up @@ -211,6 +218,7 @@
return nil
case err != nil:
b.log.Panic("initialization failed: %v", err)
return err
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this is only needed to pass the unit test that mocks log.Panic... (and doesn't actually panic, allowing the code to proceed)

}

b.run(ctx)
Expand Down Expand Up @@ -254,11 +262,25 @@
}

func (b *Builder) run(ctx context.Context) {
if err := b.generateInitialPost(ctx); err != nil {
post, metadata, err := b.generateInitialPost(ctx)
if err != nil {
b.log.Error("Failed to generate proof: %s", err)
return
}

if post != nil {
b.log.With().Info("initial post created", log.Object("post", post), log.Object("metadata", metadata))
commitmentAtxId, err := b.postSetupProvider.CommitmentAtx()
if err != nil {
b.log.With().Panic("failed to fetch commitment ATX ID.", log.Err(err))
}

Check warning on line 276 in activation/activation.go

View check run for this annotation

Codecov / codecov/patch

activation/activation.go#L275-L276 

Added lines #L275 - L276 were not covered by tests
if err := b.validator.Post(ctx, types.EpochID(0), b.nodeID, commitmentAtxId, post, metadata, b.postSetupProvider.LastOpts().NumUnits); err != nil {
events.EmitInvalidPostProof()
b.log.With().Fatal("initial POST proof is invalid. Probably initialized POST data is corrupted. Please verify the data with postcli and regenerate the corrupted files.", log.Err(err))
}

Check warning on line 280 in activation/activation.go

View check run for this annotation

Codecov / codecov/patch

activation/activation.go#L278-L280 

Added lines #L278 - L280 were not covered by tests
b.initialPost = post
poszu marked this conversation as resolved.
Show resolved Hide resolved
}

select {
case <-ctx.Done():
return
Expand All @@ -267,33 +289,35 @@
b.loop(ctx)
}

func (b *Builder) generateInitialPost(ctx context.Context) error {
func (b *Builder) generateInitialPost(ctx context.Context) (*types.Post, *types.PostMetadata, error) {
// Generate the initial POST if we don't have an ATX...
if _, err := b.cdb.GetLastAtx(b.nodeID); err == nil {
return nil
return nil, nil, nil
}
// ...and if we don't have an initial POST persisted already.
if post, err := loadPost(b.nipostBuilder.DataDir()); err == nil {
b.initialPost = post
return nil
return post, &types.PostMetadata{
Challenge: shared.ZeroChallenge,
LabelsPerUnit: b.postSetupProvider.Config().LabelsPerUnit,
}, nil
}

// Create the initial post and save it.
startTime := time.Now()
var err error
events.EmitPostStart(shared.ZeroChallenge)
b.initialPost, _, err = b.postSetupProvider.GenerateProof(ctx, shared.ZeroChallenge, proving.WithPowCreator(b.nodeID.Bytes()))
post, metadata, err := b.postSetupProvider.GenerateProof(ctx, shared.ZeroChallenge, proving.WithPowCreator(b.nodeID.Bytes()))
if err != nil {
events.EmitPostFailure()
return fmt.Errorf("post execution: %w", err)
return nil, nil, fmt.Errorf("post execution: %w", err)

Check warning on line 312 in activation/activation.go

View check run for this annotation

Codecov / codecov/patch

activation/activation.go#L312 

Added line #L312 was not covered by tests
}
events.EmitPostComplete(shared.ZeroChallenge)
metrics.PostDuration.Set(float64(time.Since(startTime).Nanoseconds()))

if err := savePost(b.nipostBuilder.DataDir(), b.initialPost); err != nil {
if err := savePost(b.nipostBuilder.DataDir(), post); err != nil {
b.log.With().Warning("failed to save initial post: %w", log.Err(err))
}
return nil
return post, metadata, nil
}

func (b *Builder) receivePendingPoetClients() *[]PoetProvingServiceClient {
Expand Down
58 changes: 45 additions & 13 deletions activation/activation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,13 @@ type testAtxBuilder struct {
coinbase types.Address
goldenATXID types.ATXID

mhdlr *MockatxHandler
mpub *mocks.MockPublisher
mnipost *MocknipostBuilder
mpost *MockpostSetupProvider
mclock *MocklayerClock
msync *Mocksyncer
mhdlr *MockatxHandler
mpub *mocks.MockPublisher
mnipost *MocknipostBuilder
mpost *MockpostSetupProvider
mclock *MocklayerClock
msync *Mocksyncer
mValidator *MocknipostValidator
}

func newTestBuilder(tb testing.TB, opts ...BuilderOption) *testAtxBuilder {
Expand All @@ -121,8 +122,11 @@ func newTestBuilder(tb testing.TB, opts ...BuilderOption) *testAtxBuilder {
mpost: NewMockpostSetupProvider(ctrl),
mclock: NewMocklayerClock(ctrl),
msync: NewMocksyncer(ctrl),
mValidator: NewMocknipostValidator(ctrl),
}

opts = append(opts, WithValidator(tab.mValidator))

cfg := Config{
CoinbaseAccount: tab.coinbase,
GoldenATXID: tab.goldenATXID,
Expand Down Expand Up @@ -255,6 +259,7 @@ func TestBuilder_StartSmeshingCoinbase(t *testing.T) {
tab.mpost.EXPECT().PrepareInitializer(gomock.Any(), gomock.Any()).AnyTimes()
tab.mpost.EXPECT().StartSession(gomock.Any()).AnyTimes()
tab.mpost.EXPECT().GenerateProof(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(&types.Post{}, &types.PostMetadata{}, nil)
tab.mValidator.EXPECT().Post(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
tab.mclock.EXPECT().AwaitLayer(gomock.Any()).Return(make(chan struct{})).AnyTimes()
require.NoError(t, tab.StartSmeshing(coinbase, postSetupOpts))
require.Equal(t, coinbase, tab.Coinbase())
Expand All @@ -271,9 +276,15 @@ func TestBuilder_RestartSmeshing(t *testing.T) {
getBuilder := func(t *testing.T) *Builder {
tab := newTestBuilder(t)
tab.mpost.EXPECT().PrepareInitializer(gomock.Any(), gomock.Any()).AnyTimes()
tab.mpost.EXPECT().CommitmentAtx().Return(types.EmptyATXID, nil).AnyTimes()
tab.mpost.EXPECT().LastOpts().Return(&PostSetupOpts{}).AnyTimes()
tab.mpost.EXPECT().StartSession(gomock.Any()).AnyTimes()
tab.mpost.EXPECT().GenerateProof(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(&types.Post{}, &types.PostMetadata{}, nil)
tab.mpost.EXPECT().GenerateProof(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(&types.Post{}, &types.PostMetadata{
Challenge: shared.ZeroChallenge,
}, nil)
tab.mpost.EXPECT().Reset().AnyTimes()
tab.mValidator.EXPECT().Post(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
tab.mpost.EXPECT().Config().AnyTimes()
ch := make(chan struct{})
close(ch)
tab.mclock.EXPECT().AwaitLayer(gomock.Any()).Return(ch).AnyTimes()
Expand Down Expand Up @@ -386,7 +397,10 @@ func TestBuilder_StopSmeshing_OnPoSTError(t *testing.T) {
tab := newTestBuilder(t)
tab.mpost.EXPECT().PrepareInitializer(gomock.Any(), gomock.Any()).AnyTimes()
tab.mpost.EXPECT().StartSession(gomock.Any()).Return(nil).AnyTimes()
tab.mpost.EXPECT().CommitmentAtx().Return(types.EmptyATXID, nil).AnyTimes()
tab.mpost.EXPECT().LastOpts().Return(&PostSetupOpts{}).AnyTimes()
tab.mpost.EXPECT().GenerateProof(gomock.Any(), gomock.Any(), gomock.Any()).Return(&types.Post{}, &types.PostMetadata{}, nil).AnyTimes()
tab.mValidator.EXPECT().Post(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
ch := make(chan struct{})
close(ch)
now := time.Now()
Expand Down Expand Up @@ -1084,7 +1098,10 @@ func TestBuilder_RetryPublishActivationTx(t *testing.T) {
func TestBuilder_InitialProofGeneratedOnce(t *testing.T) {
tab := newTestBuilder(t, WithPoetConfig(PoetConfig{PhaseShift: layerDuration * 4}))
tab.mpost.EXPECT().GenerateProof(gomock.Any(), shared.ZeroChallenge, gomock.Any()).Return(&types.Post{}, &types.PostMetadata{}, nil)
require.NoError(t, tab.generateInitialPost(context.Background()))
p, m, err := tab.generateInitialPost(context.Background())
require.NoError(t, err)
require.NotNil(t, p)
require.NotNil(t, m)

posEpoch := postGenesisEpoch + 1
challenge := newChallenge(1, types.ATXID{1, 2, 3}, types.ATXID{1, 2, 3}, posEpoch, nil)
Expand All @@ -1104,21 +1121,36 @@ func TestBuilder_InitialProofGeneratedOnce(t *testing.T) {
assertLastAtx(require.New(t), tab.nodeID, types.BytesToHash(poetByte), atx, vPrevAtx, vPrevAtx, layersPerEpoch)

// GenerateProof() should not be called again
require.NoError(t, tab.generateInitialPost(context.Background()))
p, m, err = tab.generateInitialPost(context.Background())
require.NoError(t, err)
require.Nil(t, p)
require.Nil(t, m)
}

func TestBuilder_InitialPostIsPersisted(t *testing.T) {
tab := newTestBuilder(t, WithPoetConfig(PoetConfig{PhaseShift: layerDuration * 4}))
tab.mpost.EXPECT().GenerateProof(gomock.Any(), shared.ZeroChallenge, gomock.Any()).Return(&types.Post{}, &types.PostMetadata{}, nil)
require.NoError(t, tab.generateInitialPost(context.Background()))
tab.mpost.EXPECT().Config().AnyTimes().Return(PostConfig{})
tab.mpost.EXPECT().GenerateProof(gomock.Any(), shared.ZeroChallenge, gomock.Any()).Return(&types.Post{}, &types.PostMetadata{
Challenge: shared.ZeroChallenge,
}, nil)
p, m, err := tab.generateInitialPost(context.Background())
require.NoError(t, err)
require.NotNil(t, p)
require.NotNil(t, m)
poszu marked this conversation as resolved.
Show resolved Hide resolved

// GenerateProof() should not be called again
require.NoError(t, tab.generateInitialPost(context.Background()))
p2, m2, err := tab.generateInitialPost(context.Background())
require.NoError(t, err)
require.EqualValues(t, p, p2)
require.EqualValues(t, m, m2)

// Remove the persisted post file and try again
require.NoError(t, os.Remove(filepath.Join(tab.nipostBuilder.DataDir(), postFilename)))
tab.mpost.EXPECT().GenerateProof(gomock.Any(), shared.ZeroChallenge, gomock.Any()).Return(&types.Post{}, &types.PostMetadata{}, nil)
require.NoError(t, tab.generateInitialPost(context.Background()))
p, m, err = tab.generateInitialPost(context.Background())
require.NoError(t, err)
require.NotNil(t, p)
require.NotNil(t, m)
}

func TestBuilder_UpdatePoets(t *testing.T) {
Expand Down
35 changes: 34 additions & 1 deletion activation/nipost.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"github.com/spacemeshos/merkle-tree"
"github.com/spacemeshos/poet/shared"
"github.com/spacemeshos/post/proving"
"github.com/spacemeshos/post/verifying"
"golang.org/x/sync/errgroup"

"github.com/spacemeshos/go-spacemesh/activation/metrics"
Expand Down Expand Up @@ -69,6 +70,15 @@
signer *signing.EdSigner
layerClock layerClock
poetCfg PoetConfig
validator nipostValidator
}

type NIPostBuilderOption func(*NIPostBuilder)

func WithNipostValidator(v nipostValidator) NIPostBuilderOption {
return func(nb *NIPostBuilder) {
nb.validator = v
}
}

type poetDbAPI interface {
Expand All @@ -87,8 +97,9 @@
signer *signing.EdSigner,
poetCfg PoetConfig,
layerClock layerClock,
opts ...NIPostBuilderOption,
) *NIPostBuilder {
return &NIPostBuilder{
b := &NIPostBuilder{
nodeID: nodeID,
postSetupProvider: postSetupProvider,
poetProvers: poetProvers,
Expand All @@ -100,6 +111,11 @@
poetCfg: poetCfg,
layerClock: layerClock,
}

for _, opt := range opts {
opt(b)
}
return b
}

func (nb *NIPostBuilder) DataDir() string {
Expand Down Expand Up @@ -217,6 +233,23 @@
events.EmitPostFailure()
return nil, 0, fmt.Errorf("failed to generate Post: %v", err)
}
commitmentAtxId, err := nb.postSetupProvider.CommitmentAtx()
if err != nil {
return nil, 0, fmt.Errorf("failed to get commitment ATX: %v", err)
}

Check warning on line 239 in activation/nipost.go

View check run for this annotation

Codecov / codecov/patch

activation/nipost.go#L238-L239 

Added lines #L238 - L239 were not covered by tests
if err := nb.validator.Post(
ctx,
challenge.PublishEpoch,
nb.nodeID,
commitmentAtxId,
proof,
proofMetadata,
nb.postSetupProvider.LastOpts().NumUnits,
verifying.WithLabelScryptParams(nb.postSetupProvider.LastOpts().Scrypt),
); err != nil {
events.EmitInvalidPostProof()
return nil, 0, fmt.Errorf("failed to verify Post: %v", err)
}

Check warning on line 252 in activation/nipost.go

View check run for this annotation

Codecov / codecov/patch

activation/nipost.go#L250-L252 

Added lines #L250 - L252 were not covered by tests
events.EmitPostComplete(nb.state.PoetProofRef[:])
postGenDuration = time.Since(startTime)
nb.log.With().Info("finished post execution", log.Duration("duration", postGenDuration))
Expand Down