diff --git a/internal/core/execute/v2/block/exec_process.go b/internal/core/execute/v2/block/exec_process.go index d4593119a..cd12f0d12 100644 --- a/internal/core/execute/v2/block/exec_process.go +++ b/internal/core/execute/v2/block/exec_process.go @@ -46,7 +46,7 @@ func (b *Block) Process(envelope *messaging.Envelope) ([]*protocol.TransactionSt } // Make sure every transaction is signed - err = checkForUnsignedTransactions(messages) + err = b.Executor.checkForUnsignedTransactions(messages) if err != nil { return nil, errors.UnknownError.Wrap(err) } @@ -229,26 +229,49 @@ func (d *bundle) process() ([]*protocol.TransactionStatus, error) { // checkForUnsignedTransactions returns an error if the message bundle includes // any unsigned transactions. -func checkForUnsignedTransactions(messages []messaging.Message) error { - unsigned := set[[32]byte]{} +func (x *Executor) checkForUnsignedTransactions(messages []messaging.Message) error { + // Record signatures and transactions + haveSigFor := map[[32]byte]bool{} + haveTxn := map[[32]byte]bool{} for _, msg := range messages { - if msg, ok := msg.(*messaging.TransactionMessage); ok { - unsigned[msg.ID().Hash()] = struct{}{} + if x.globals.Active.ExecutorVersion.V2BaikonurEnabled() { + if _, ok := msg.(*messaging.BlockAnchor); ok { + continue + } + } + + if msg, ok := messaging.UnwrapAs[messaging.MessageForTransaction](msg); ok { + // User signatures can be sent without a transaction, but authority + // signatures and other message-for-transaction types cannot + sig, ok := msg.(*messaging.SignatureMessage) + isUser := ok && sig.Signature.Type() != protocol.SignatureTypeAuthority + haveSigFor[msg.GetTxID().Hash()] = isUser + } + + if txn, ok := msg.(*messaging.TransactionMessage); ok { + isRemote := txn.GetTransaction().Body.Type() == protocol.TransactionTypeRemote + haveTxn[txn.ID().Hash()] = isRemote } } - for _, msg := range messages { - again: - switch m := msg.(type) { - case messaging.MessageForTransaction: - delete(unsigned, m.GetTxID().Hash()) - case interface{ Unwrap() messaging.Message }: - msg = m.Unwrap() - goto again + + // If a transaction does not have a signature, reject the bundle + for txn := range haveTxn { + if _, has := haveSigFor[txn]; !has { + return errors.BadRequest.With("message bundle includes an unsigned transaction") } } - if len(unsigned) > 0 { - return errors.BadRequest.With("message bundle includes an unsigned transaction") + + // If an authority signature or other synthetic message-for-transaction is + // sent without its transaction, reject the bundle + if x.globals.Active.ExecutorVersion.V2BaikonurEnabled() { + for txn, isUserSig := range haveSigFor { + isRemote, has := haveTxn[txn] + if !isUserSig && (!has || isRemote) { + return errors.BadRequest.With("message bundle is missing a transaction") + } + } } + return nil } diff --git a/internal/core/execute/v2/block/exec_validate.go b/internal/core/execute/v2/block/exec_validate.go index edfc01018..5f12d0099 100644 --- a/internal/core/execute/v2/block/exec_validate.go +++ b/internal/core/execute/v2/block/exec_validate.go @@ -28,7 +28,7 @@ func (x *Executor) Validate(envelope *messaging.Envelope, _ bool) ([]*protocol.T } // Make sure every transaction is signed - err = checkForUnsignedTransactions(messages) + err = x.checkForUnsignedTransactions(messages) if err != nil { return nil, errors.UnknownError.Wrap(err) } diff --git a/internal/core/execute/v2/block/utils.go b/internal/core/execute/v2/block/utils.go index b7bba02d7..4d72c23cc 100644 --- a/internal/core/execute/v2/block/utils.go +++ b/internal/core/execute/v2/block/utils.go @@ -17,21 +17,6 @@ import ( "gitlab.com/accumulatenetwork/accumulate/protocol" ) -// none is an empty struct. -type none = struct{} - -// set is an unordered set of T implemented as a map of T to [none]. -type set[T comparable] map[T]none - -// Add adds a value to the set. -func (s set[T]) Add(v T) { s[v] = none{} } - -// Remove removes a value from the set. -func (s set[T]) Remove(v T) { delete(s, v) } - -// Hash checks if the set has the given value. -func (s set[T]) Has(v T) bool { _, ok := s[v]; return ok } - // orderedMap is an ordered map from K to V implemented with a builtin map, // slice of keys, and comparison function. type orderedMap[K comparable, V any] struct { diff --git a/test/e2e/regression2_test.go b/test/e2e/regression2_test.go index db17e1ef1..d3dac3cdd 100644 --- a/test/e2e/regression2_test.go +++ b/test/e2e/regression2_test.go @@ -8,6 +8,7 @@ package e2e import ( "bytes" + "context" "crypto/sha256" "fmt" "math/big" @@ -1137,3 +1138,85 @@ func TestChainUpdateAnchor(t *testing.T) { // Verify the anchor matches the receipt require.Equal(t, r1.Receipt.LocalBlock, anchor.MinorBlockIndex) } + +func TestAuthoritySignatureWithoutTransaction(t *testing.T) { + cases := []struct { + Version ExecutorVersion + Fails bool + }{ + {Version: ExecutorVersionV2, Fails: false}, + {Version: ExecutorVersionLatest, Fails: true}, + } + + for _, c := range cases { + t.Run(c.Version.String(), func(t *testing.T) { + alice := url.MustParse("alice") + aliceKey := acctesting.GenerateKey(alice) + + // Capture the authority signature + var captured *messaging.Envelope + capFn := func(_ context.Context, _ *url.URL, env *messaging.Envelope) (send bool, err error) { + for _, m := range env.Messages { + syn, ok := m.(interface{ Data() *messaging.SynthFields }) + if !ok { + continue + } + seq, ok := syn.Data().Message.(*messaging.SequencedMessage) + if !ok { + continue + } + sig, ok := seq.Message.(*messaging.SignatureMessage) + if !ok || sig.Signature.Type() != SignatureTypeAuthority { + continue + } + require.Nil(t, captured) + captured = env + return false, nil + } + return true, nil + } + + // Initialize + sim := NewSim(t, + simulator.SimpleNetwork(t.Name(), 1, 1), + simulator.GenesisWithVersion(GenesisTime, c.Version), + simulator.CaptureDispatchedMessages(capFn), + ) + + MakeIdentity(t, sim.DatabaseFor(alice), alice, aliceKey[32:]) + CreditCredits(t, sim.DatabaseFor(alice), alice.JoinPath("book", "1"), 1e9) + + // Execute + sim.BuildAndSubmitTxnSuccessfully( + build.Transaction().For("bob.acme", "tokens"). + BurnTokens(1, 0). + SignWith(alice, "book", "1").Version(1).Timestamp(1).PrivateKey(aliceKey)) + + sim.StepUntil( + True(func(h *Harness) bool { return captured != nil })) + + // Remove the transaction + msgs, err := captured.Normalize() + require.NoError(t, err) + captured = new(messaging.Envelope) + var removed int + for _, msg := range msgs { + if msg.Type() == messaging.MessageTypeTransaction { + removed++ + } else { + captured.Messages = append(captured.Messages, msg) + } + } + require.NotZero(t, removed) + + // Resubmit + _, err = sim.SubmitTo("BVN0", captured) + if c.Fails { + require.Error(t, err) + require.Contains(t, err.Error(), "message bundle is missing a transaction") + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/test/simulator/dispatcher.go b/test/simulator/dispatcher.go index 03ddf0d26..46cebfbd5 100644 --- a/test/simulator/dispatcher.go +++ b/test/simulator/dispatcher.go @@ -33,20 +33,30 @@ func (fakeDispatcher) Send(context.Context) <-chan error { return ch } +type dispatchInterceptor = func(ctx context.Context, u *url.URL, env *messaging.Envelope) (send bool, err error) + // dispatcher implements [block.Dispatcher] for the simulator. // // dispatcher maintains a separate bundle (slice) of messages for each call to // Submit to make it easier to write tests that drop certain messages. type dispatcher struct { - client *services.Network - router routing.Router - envelopes map[string][]*messaging.Envelope + client *services.Network + router routing.Router + envelopes map[string][]*messaging.Envelope + interceptor dispatchInterceptor } var _ execute.Dispatcher = (*dispatcher)(nil) // Submit routes the envelope and adds it to the queue for a partition. func (d *dispatcher) Submit(ctx context.Context, u *url.URL, env *messaging.Envelope) error { + if d.interceptor != nil { + keep, err := d.interceptor(ctx, u, env) + if !keep || err != nil { + return err + } + } + partition, err := d.router.RouteAccount(u) if err != nil { return err diff --git a/test/simulator/factory.go b/test/simulator/factory.go index 494981c29..ef75f5921 100644 --- a/test/simulator/factory.go +++ b/test/simulator/factory.go @@ -44,11 +44,12 @@ type simFactory struct { recordings RecordingFunc abci abciFunc - dropDispatchedMessages bool - skipProposalCheck bool - ignoreDeliverResults bool - ignoreCommitResults bool - deterministic bool + dropDispatchedMessages bool + skipProposalCheck bool + ignoreDeliverResults bool + ignoreCommitResults bool + deterministic bool + interceptDispatchedMessages dispatchInterceptor // State logger log.Logger @@ -298,12 +299,14 @@ func (f *simFactory) getDispatcherFunc() func() execute.Dispatcher { // Avoid capture services := f.getServices() router := f.getRouter() + interceptor := f.interceptDispatchedMessages f.dispatcherFunc = func() execute.Dispatcher { d := new(dispatcher) d.client = services d.router = router d.envelopes = map[string][]*messaging.Envelope{} + d.interceptor = interceptor return d } return f.dispatcherFunc diff --git a/test/simulator/options.go b/test/simulator/options.go index 26ead7c52..365f57603 100644 --- a/test/simulator/options.go +++ b/test/simulator/options.go @@ -52,6 +52,15 @@ func DropDispatchedMessages(opts *simFactory) error { return nil } +// CaptureDispatchedMessages allows the caller to capture internally dispatched +// messages. +func CaptureDispatchedMessages(fn dispatchInterceptor) Option { + return func(opts *simFactory) error { + opts.interceptDispatchedMessages = fn + return nil + } +} + // SkipProposalCheck skips checking if each non-leader node agrees with the // leader's proposed block. func SkipProposalCheck(opts *simFactory) error {