Spend a hashcash stamp only when the request succeeds (closes #158)

A session request refused with 400 or 409, or a +H channel message
refused for membership, a ban or +m, used to spend its stamp, so a
retry with the same stamp was refused as already used.

hashcash.Validator.Validate is split into Check, which spends
nothing, and Claim, which records the stamp; Release undoes a claim.
Session creation checks the stamp first, claims it just before
CreateSession and releases it if CreateSession fails.
SendChannelMessage checks the stamp after the membership, ban and +m
checks.

Model: opus-5-5

README.md

@@ -3161,12 +3161,13 @@ creating one session pays once and keeps their session.
 ### Why Hashcash and Not Rate Limits?
 
 - **Little state to track**: No IP tables, no token buckets, no sliding windows.
-  The server verifies a single hash and remembers the stamps already spent.
+  The server hashes the stamp and remembers the stamps already spent.
 - **Works through NATs and proxies**: Doesn't punish shared IPs (university
   campuses, corporate networks, Tor exits). Every client computes their own
   proof independently.
 - **Cost falls on the requester**: The server's verification cost is constant
-  (one SHA-256 hash) regardless of difficulty. Only the client does more work.
+  regardless of difficulty: at most two SHA-256 hashes of the stamp per session
+  request, one to check it and one to spend it. Only the client does more work.
 - **Fits the "no accounts" philosophy**: Proof-of-work is the cost of entry. No
   registration, no email, no phone number, no CAPTCHA. Just compute.
 - **Language-agnostic**: SHA-256 is available in every programming language. The

internal/handlers/api_test.go

@@ -4122,9 +4122,14 @@ func sessionCount(t *testing.T, tserver *testServer) int64 {
 // before its nick is looked at, that a request refused with
 // 400 or 409 does not spend its stamp, so the same stamp
 // then creates a session, that the stamp stays spent after
-// that, and that no refused request creates a session.
+// that, that a spent stamp with an invalid nick is refused
+// for its nick, and that no refused request creates a
+// session.
 func TestRefusedSessionLeavesStampUnspent(t *testing.T) {
-	const bits = 2
+	const (
+		bits    = 2
+		badNick = "bad nick"
+	)
 
 	tserver := newTestServerWith(t, bits, "", "")
 
@@ -4143,14 +4148,16 @@ func TestRefusedSessionLeavesStampUnspent(t *testing.T) {
 		nick, username, stamp string
 		want                  int
 	}{
-		{"bad nick", "", otherServerStamp, http.StatusPaymentRequired},
-		{"bad nick", "", stamp, http.StatusBadRequest},
+		{badNick, "", otherServerStamp, http.StatusPaymentRequired},
+		{badNick, "", stamp, http.StatusBadRequest},
 		{"fresh", "bad username", stamp, http.StatusBadRequest},
 		{"taken", "", stamp, http.StatusConflict},
 		{"fresh", "", stamp, http.StatusCreated},
 		{"another", "", stamp, http.StatusPaymentRequired},
 		// Refused as already spent, the stamp is not given back.
 		{"later", "", stamp, http.StatusPaymentRequired},
+		// Whether the stamp is spent is looked at after the nick.
+		{badNick, "", stamp, http.StatusBadRequest},
 		// The refused request for "another" left the nick free.
 		{"another", "", api.MintHashcash(bits, "neoirc"), http.StatusCreated},
 	}

internal/hashcash/hashcash_test.go

@@ -111,7 +111,8 @@ func TestReplayDetection(t *testing.T) {
 }
 
 // TestCheckDoesNotSpendStamp checks that a stamp Check
-// accepts can still be claimed, and only once.
+// accepts can still be claimed, and only once, and that
+// Check still accepts it once it is spent.
 func TestCheckDoesNotSpendStamp(t *testing.T) {
 	t.Parallel()
 
@@ -136,6 +137,11 @@ func TestCheckDoesNotSpendStamp(t *testing.T) {
 	if err == nil {
 		t.Fatal("replay not detected")
 	}
+
+	err = validator.Check(stamp, testBits)
+	if err != nil {
+		t.Fatalf("spent stamp rejected by Check: %v", err)
+	}
 }
 
 // TestReleasedStampCanBeClaimedAgain checks that Release

internal/ircserver/conn.go

@@ -70,7 +70,6 @@ type Conn struct {
 }
 
 func newConn(
-	ctx context.Context,
 	tcpConn net.Conn,
 	log *slog.Logger,
 	database *db.Database,
@@ -94,7 +93,6 @@ func newConn(
 		svc:       svc,
 		serverSfx: srvName,
 		remoteIP:  host,
-		hostname:  resolveHost(ctx, host),
 	}
 
 	conn.commands = conn.buildCommandMap()
@@ -212,12 +210,15 @@ func resolveHost(ctx context.Context, addr string) string {
 }
 
 // serve is the main loop for a single IRC client connection.
+// It first sets the client's hostname to what resolve returns
+// for the client's address.
 // Cancelling ctx when it returns stops the relay goroutine.
 // When nothing has been read for interval the client is sent
 // a PING, and when nothing is read within deadline after that
 // the connection is closed.
 func (c *Conn) serve(
 	ctx context.Context,
+	resolve func(ctx context.Context, addr string) string,
 	interval, deadline time.Duration,
 ) {
 	ctx, cancel := context.WithCancel(ctx)
@@ -226,6 +227,11 @@ func (c *Conn) serve(
 	// still end the session then.
 	defer c.cleanup(context.WithoutCancel(ctx))
 
+	// The hostname is looked up here, on this connection's
+	// goroutine, and not when the connection is accepted, so
+	// that a slow lookup holds up only this client.
+	c.hostname = resolve(ctx, c.remoteIP)
+
 	ping := time.AfterFunc(interval, func() {
 		c.send("PING :" + c.serverSfx)
 	})

internal/ircserver/export_test.go

@@ -15,7 +15,8 @@ import (
 
 // NewTestServer creates a Server suitable for testing, whose
 // connections use interval and deadline in place of
-// pingInterval and pongDeadline.
+// pingInterval and pongDeadline, and resolve in place of
+// resolveHost.
 // The caller must call Stop() when finished.
 func NewTestServer(
 	log *slog.Logger,
@@ -23,6 +24,7 @@ func NewTestServer(
 	database *db.Database,
 	brk *broker.Broker,
 	interval, deadline time.Duration,
+	resolve func(ctx context.Context, addr string) string,
 ) *Server {
 	globs := &globals.Globals{
 		Appname:   "neoirc",
@@ -44,9 +46,16 @@ func NewTestServer(
 
 		pingInterval: interval,
 		pongDeadline: deadline,
+		resolveHost:  resolve,
 	}
 }
 
+// ResolveHost is the reverse DNS lookup that a server made by
+// New does for each client.
+func ResolveHost(ctx context.Context, addr string) string {
+	return resolveHost(ctx, addr)
+}
+
 // Start exposes the unexported start method for tests.
 func (s *Server) Start(addr string) error {
 	return s.start(context.Background(), addr)
@@ -97,10 +106,7 @@ func NewTestConn(
 	tcpConn net.Conn,
 	nick string,
 ) *Conn {
-	conn := newConn(
-		context.Background(), tcpConn, log,
-		nil, nil, cfg, nil,
-	)
+	conn := newConn(tcpConn, log, nil, nil, cfg, nil)
 	conn.nick = nick
 
 	return conn

internal/ircserver/server.go

@@ -52,6 +52,10 @@ type Server struct {
 	// same names, except in tests, which shorten them.
 	pingInterval time.Duration
 	pongDeadline time.Duration
+
+	// resolveHost is the function of the same name, except in
+	// tests, which replace it with one that blocks.
+	resolveHost func(ctx context.Context, addr string) string
 }
 
 // New creates a new IRC Server and registers its lifecycle
@@ -75,6 +79,7 @@ func New(
 
 		pingInterval: pingInterval,
 		pongDeadline: pongDeadline,
+		resolveHost:  resolveHost,
 	}
 
 	listenAddr := params.Config.IRCListenAddr
@@ -175,7 +180,7 @@ func (s *Server) acceptLoop(ctx context.Context) {
 		}
 
 		client := newConn(
-			ctx, tcpConn, s.log,
+			tcpConn, s.log,
 			s.database, s.brk, s.cfg, s.svc,
 		)
 
@@ -204,7 +209,10 @@ func (s *Server) acceptLoop(ctx context.Context) {
 				s.mu.Unlock()
 			}()
 
-			client.serve(ctx, s.pingInterval, s.pongDeadline)
+			client.serve(
+				ctx, s.resolveHost,
+				s.pingInterval, s.pongDeadline,
+			)
 		}()
 	}
 }

internal/ircserver/server_test.go

@@ -2,6 +2,7 @@ package ircserver_test
 
 import (
 	"bufio"
+	"context"
 	"crypto/rand"
 	"database/sql"
 	"errors"
@@ -11,6 +12,7 @@ import (
 	"os"
 	"runtime"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -90,6 +92,21 @@ func newTestEnvWithPingTimes(
 ) *testEnv {
 	t.Helper()
 
+	return newTestEnvWithResolver(
+		t, cfg, interval, deadline, ircserver.ResolveHost,
+	)
+}
+
+// newTestEnvWithResolver is newTestEnvWithPingTimes, except
+// that the server looks up a client's hostname with resolve.
+func newTestEnvWithResolver(
+	t *testing.T,
+	cfg *config.Config,
+	interval, deadline time.Duration,
+	resolve func(ctx context.Context, addr string) string,
+) *testEnv {
+	t.Helper()
+
 	// A random name, so that no other test environment, not
 	// even an earlier run of the same test, can share it.
 	dsn := fmt.Sprintf(
@@ -147,7 +164,7 @@ func newTestEnvWithPingTimes(
 	))
 
 	srv := ircserver.NewTestServer(
-		log, cfg, database, brk, interval, deadline,
+		log, cfg, database, brk, interval, deadline, resolve,
 	)
 
 	err = srv.Start(addr)
@@ -415,6 +432,49 @@ func TestRegistration(t *testing.T) {
 	assertContains(t, lines, " 001 ", "RPL_WELCOME")
 }
 
+// TestRegistrationWhileAnotherLookupIsPending checks that a
+// client is accepted and registered while the hostname lookup
+// of an earlier client has not returned, and that it
+// registers with the hostname its own lookup returned.
+func TestRegistrationWhileAnotherLookupIsPending(t *testing.T) {
+	t.Parallel()
+
+	const lookedUpHost = "client.example"
+
+	var lookups atomic.Int32
+
+	firstLookupStarted := make(chan struct{})
+
+	// The first lookup returns only when the server stops.
+	resolve := func(ctx context.Context, _ string) string {
+		if lookups.Add(1) == 1 {
+			close(firstLookupStarted)
+			<-ctx.Done()
+		}
+
+		return lookedUpHost
+	}
+
+	env := newTestEnvWithResolver(
+		t, &config.Config{ServerName: testServerName},
+		ircserver.PingInterval, ircserver.PongDeadline, resolve,
+	)
+
+	env.dial(t)
+
+	select {
+	case <-firstLookupStarted:
+	case <-time.After(testTimeout):
+		t.Fatal("the first client's lookup did not start")
+	}
+
+	lines := env.dial(t).register("second")
+	assertContains(
+		t, lines, "second!second@"+lookedUpHost,
+		"RPL_WELCOME with the looked-up hostname",
+	)
+}
+
 // TestEachTestEnvHasItsOwnDatabase checks that a test
 // environment starts on an empty database while another one
 // is still open.