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,13 +3161,12 @@ 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 hashes the stamp and remembers the stamps already spent.
+  The server verifies a single hash 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
-  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.
+  (one SHA-256 hash) regardless of difficulty. 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,14 +4122,9 @@ 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, that a spent stamp with an invalid nick is refused
-// for its nick, and that no refused request creates a
-// session.
+// that, and that no refused request creates a session.
 func TestRefusedSessionLeavesStampUnspent(t *testing.T) {
-	const (
-		bits    = 2
-		badNick = "bad nick"
-	)
+	const bits = 2
 
 	tserver := newTestServerWith(t, bits, "", "")
 
@@ -4148,16 +4143,14 @@ func TestRefusedSessionLeavesStampUnspent(t *testing.T) {
 		nick, username, stamp string
 		want                  int
 	}{
-		{badNick, "", otherServerStamp, http.StatusPaymentRequired},
-		{badNick, "", stamp, http.StatusBadRequest},
+		{"bad nick", "", otherServerStamp, http.StatusPaymentRequired},
+		{"bad nick", "", 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,8 +111,7 @@ func TestReplayDetection(t *testing.T) {
 }
 
 // TestCheckDoesNotSpendStamp checks that a stamp Check
-// accepts can still be claimed, and only once, and that
-// Check still accepts it once it is spent.
+// accepts can still be claimed, and only once.
 func TestCheckDoesNotSpendStamp(t *testing.T) {
 	t.Parallel()
 
@@ -137,11 +136,6 @@ 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,6 +70,7 @@ type Conn struct {
 }
 
 func newConn(
+	ctx context.Context,
 	tcpConn net.Conn,
 	log *slog.Logger,
 	database *db.Database,
@@ -93,6 +94,7 @@ func newConn(
 		svc:       svc,
 		serverSfx: srvName,
 		remoteIP:  host,
+		hostname:  resolveHost(ctx, host),
 	}
 
 	conn.commands = conn.buildCommandMap()
@@ -210,15 +212,12 @@ 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)
@@ -227,11 +226,6 @@ 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,8 +15,7 @@ import (
 
 // NewTestServer creates a Server suitable for testing, whose
 // connections use interval and deadline in place of
-// pingInterval and pongDeadline, and resolve in place of
-// resolveHost.
+// pingInterval and pongDeadline.
 // The caller must call Stop() when finished.
 func NewTestServer(
 	log *slog.Logger,
@@ -24,7 +23,6 @@ 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",
@@ -46,16 +44,9 @@ 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)
@@ -106,7 +97,10 @@ func NewTestConn(
 	tcpConn net.Conn,
 	nick string,
 ) *Conn {
-	conn := newConn(tcpConn, log, nil, nil, cfg, nil)
+	conn := newConn(
+		context.Background(), tcpConn, log,
+		nil, nil, cfg, nil,
+	)
 	conn.nick = nick
 
 	return conn

internal/ircserver/server.go

@@ -52,10 +52,6 @@ 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
@@ -79,7 +75,6 @@ func New(
 
 		pingInterval: pingInterval,
 		pongDeadline: pongDeadline,
-		resolveHost:  resolveHost,
 	}
 
 	listenAddr := params.Config.IRCListenAddr
@@ -180,7 +175,7 @@ func (s *Server) acceptLoop(ctx context.Context) {
 		}
 
 		client := newConn(
-			tcpConn, s.log,
+			ctx, tcpConn, s.log,
 			s.database, s.brk, s.cfg, s.svc,
 		)
 
@@ -209,10 +204,7 @@ func (s *Server) acceptLoop(ctx context.Context) {
 				s.mu.Unlock()
 			}()
 
-			client.serve(
-				ctx, s.resolveHost,
-				s.pingInterval, s.pongDeadline,
-			)
+			client.serve(ctx, s.pingInterval, s.pongDeadline)
 		}()
 	}
 }

internal/ircserver/server_test.go

@@ -2,7 +2,6 @@ package ircserver_test
 
 import (
 	"bufio"
-	"context"
 	"crypto/rand"
 	"database/sql"
 	"errors"
@@ -12,7 +11,6 @@ import (
 	"os"
 	"runtime"
 	"strings"
-	"sync/atomic"
 	"testing"
 	"time"
 
@@ -92,21 +90,6 @@ 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(
@@ -164,7 +147,7 @@ func newTestEnvWithResolver(
 	))
 
 	srv := ircserver.NewTestServer(
-		log, cfg, database, brk, interval, deadline, resolve,
+		log, cfg, database, brk, interval, deadline,
 	)
 
 	err = srv.Start(addr)
@@ -432,49 +415,6 @@ 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.