From e489f3f3d13898ec1ba0f6a9451cc066bac314d0 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 20 Oct 2026 20:31:27 +0200 Subject: [PATCH] Spend a channel hashcash stamp in one statement (closes #150) checkChannelHashcash looked a stamp up with one statement and recorded it with another, so concurrent requests carrying the same stamp could all pass the lookup before any of them recorded it, and each message was sent. RecordSpentHashcash now reports whether its INSERT OR IGNORE added a row, and the message is refused when it did not. A single statement is atomic in SQLite. IsHashcashSpent had no other caller and is removed. A failed write of the stamp now refuses the message instead of being logged and ignored. Model: opus-5-5 --- internal/db/queries.go | 32 +++------ internal/handlers/api_test.go | 131 ++++++++++++++++++++++++++++++++++ internal/service/service.go | 18 ++--- 3 files changed, 147 insertions(+), 34 deletions(-) diff --git a/internal/db/queries.go b/internal/db/queries.go index be27ff3..655fa04 100644 --- a/internal/db/queries.go +++ b/internal/db/queries.go @@ -1783,45 +1783,33 @@ func (database *Database) SetChannelHashcashBits( } // RecordSpentHashcash stores a spent hashcash stamp hash -// for replay prevention. +// for replay prevention. It returns false when the hash was +// already stored. The one INSERT is both the check and the +// record, so of several concurrent callers with the same +// hash exactly one gets true. func (database *Database) RecordSpentHashcash( ctx context.Context, stampHash string, -) error { - _, err := database.conn.ExecContext(ctx, +) (bool, error) { + res, err := database.conn.ExecContext(ctx, `INSERT OR IGNORE INTO spent_hashcash (stamp_hash, created_at) VALUES (?, ?)`, stampHash, time.Now()) if err != nil { - return fmt.Errorf( + return false, fmt.Errorf( "record spent hashcash: %w", err, ) } - return nil -} - -// IsHashcashSpent checks whether a hashcash stamp hash -// has already been used. -func (database *Database) IsHashcashSpent( - ctx context.Context, - stampHash string, -) (bool, error) { - var count int - - err := database.conn.QueryRowContext(ctx, - `SELECT COUNT(*) FROM spent_hashcash - WHERE stamp_hash = ?`, - stampHash, - ).Scan(&count) + inserted, err := res.RowsAffected() if err != nil { return false, fmt.Errorf( - "check spent hashcash: %w", err, + "record spent hashcash: %w", err, ) } - return count > 0, nil + return inserted > 0, nil } // PruneSpentHashcash deletes spent hashcash tokens older diff --git a/internal/handlers/api_test.go b/internal/handlers/api_test.go index 31861f3..fca94c9 100644 --- a/internal/handlers/api_test.go +++ b/internal/handlers/api_test.go @@ -3368,6 +3368,137 @@ func TestChannelHashcashRejectReplayedStamp(t *testing.T) { } } +// sendConcurrently posts cmd in the given number of requests +// at once and returns the status field of each response. +func (tserver *testServer) sendConcurrently( + token string, cmd map[string]any, requests int, +) []any { + tserver.t.Helper() + + body, err := json.Marshal(cmd) + if err != nil { + tserver.t.Fatalf("marshal command: %v", err) + } + + // Each request writes only its own entry. + statuses := make([]any, requests) + start := make(chan struct{}) + + var waitGroup sync.WaitGroup + + for idx := range requests { + waitGroup.Add(1) + + go func() { + defer waitGroup.Done() + + <-start + + resp, reqErr := doRequestAuth( + tserver.t, http.MethodPost, + tserver.url(apiMessages), token, + bytes.NewReader(body), + ) + if reqErr != nil { + tserver.t.Error(reqErr) + + return + } + + defer func() { _ = resp.Body.Close() }() + + var result map[string]any + + _ = json.NewDecoder(resp.Body).Decode(&result) + + statuses[idx] = result[statusKey] + }() + } + + close(start) + waitGroup.Wait() + + return statuses +} + +// TestChannelHashcashRejectConcurrentReplay sends one stamp +// in several requests at once and checks that exactly one is +// accepted and the other member receives exactly one copy. +func TestChannelHashcashRejectConcurrentReplay(t *testing.T) { + const ( + channel = "#hcrace" + text = "one stamp" + sender = "hcrace_sender" + requests = 16 + ) + + tserver := newTestServer(t) + senderToken := tserver.createSession(sender) + receiverToken := tserver.createSession("hcrace_receiver") + + tserver.sendCommand(senderToken, map[string]any{ + commandKey: joinCmd, toKey: channel, + }) + tserver.sendCommand(receiverToken, map[string]any{ + commandKey: joinCmd, toKey: channel, + }) + + tserver.sendCommand(senderToken, map[string]any{ + commandKey: modeCmd, + toKey: channel, + bodyKey: []string{"+H", "2"}, + }) + + _, lastID := tserver.pollMessages(receiverToken, 0) + + msgBody, marshalErr := json.Marshal([]string{text}) + if marshalErr != nil { + t.Fatal(marshalErr) + } + + stamp := mintTestChannelHashcash(t, 2, channel, msgBody) + + statuses := tserver.sendConcurrently(senderToken, map[string]any{ + commandKey: privmsgCmd, + toKey: channel, + bodyKey: []string{text}, + metaKey: map[string]any{hashcashKey: stamp}, + }, requests) + + accepted := 0 + + for _, status := range statuses { + if status == "sent" { + accepted++ + } + } + + if accepted != 1 { + t.Fatalf( + "expected 1 accepted message, got %d: %v", + accepted, statuses, + ) + } + + msgs, _ := tserver.pollMessages(receiverToken, lastID) + + delivered := 0 + + for _, msg := range msgs { + if msg[commandKey] == privmsgCmd && + msg["from"] == sender { + delivered++ + } + } + + if delivered != 1 { + t.Fatalf( + "expected 1 delivered copy, got %d: %v", + delivered, msgs, + ) + } +} + func TestChannelHashcashNoRequirementWorks(t *testing.T) { tserver := newTestServer(t) token := tserver.createSession("hcnone_user") diff --git a/internal/service/service.go b/internal/service/service.go index 4a43946..2977aa9 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -1285,8 +1285,9 @@ func (s *Service) requireOper( // checkChannelHashcash returns ERR_CANNOTSENDTOCHAN unless // the channel has no +H, or meta carries a hashcash stamp // for this channel and body with enough bits that has not -// been used before. A stamp that passes is recorded as -// spent. +// been used before. Recording the stamp as spent is the +// check that it is unused, so a stamp sent in several +// concurrent requests passes in only one of them. func (s *Service) checkChannelHashcash( ctx context.Context, chID int64, @@ -1324,12 +1325,12 @@ func (s *Service) checkChannelHashcash( stampKey := hashcash.StampHash(stamp) - spent, err := s.db.IsHashcashSpent(ctx, stampKey) + recorded, err := s.db.RecordSpentHashcash(ctx, stampKey) if err != nil { - return fmt.Errorf("check spent hashcash: %w", err) + return fmt.Errorf("record spent hashcash: %w", err) } - if spent { + if !recorded { return &IRCError{ irc.ErrCannotSendToChan, []string{channel}, @@ -1337,13 +1338,6 @@ func (s *Service) checkChannelHashcash( } } - err = s.db.RecordSpentHashcash(ctx, stampKey) - if err != nil { - s.log.Error( - "record spent hashcash", "error", err, - ) - } - return nil }