feat(validator): refuse to sign a slot a key already signed

[adust09]

Closes #1203.

sign() requires that a key never sign two different messages for one slot, but only an in-memory set upheld it — and the proposal path had no per-slot guard at all. A crash-restart, a backward clock step, or a restore re-ran the duty against a possibly different head and signed twice under the same one-time key.

Records the highest slot each key has signed and refuses slot <= last_signed_slot at the signing boundary.

• New signing_records table keyed by (validator_index, key_role), never pruned.
• The claim commits before the signature: an interruption forfeits the duty, not the key.
• Roles tracked separately, so proposing and attesting in one slot stays legal.
• Without a database, records are process-local and run() warns.
• A refusal skips one duty; _attested_slots stays as the duty loop's dedup.

Concurrent nodes sharing a key keep separate records, so exclusive key-store access remains an operational requirement.

17 new tests, including a restart scenario: sign slot 1, reopen the database, replacement service refused. 2961 passed, coverage 91.79%.

[gopikannappan]

Hello, I am new around here, so please take this as one pair of eyes rather than a verdict. I came at it from the leanVM side while working on #1209, got curious, and since this has been sitting a while I thought a read might be more use than another issue comment.

Short version: it looks right to me, the 17 tests pass on e8f9c29d, and I learned something from reading it.

The ordering is the bit I would have got wrong. Claiming the slot before signing means an interruption costs a duty rather than a key, and that is the only way round that is safe. The one-line comment saying so is doing a lot of work.

I wanted to see the restart behaviour for myself rather than take the test's word for it, so I reserved slot 5, threw the object away, and built a fresh SigningProtection over the same database. It refuses. That is the issue closed as far as I can see.

I also tried breaking it in two ways to see whether your tests bite, and they do:

What I changed	Result
slot <= last_signed weakened to slot < last_signed, so the same slot is allowed again	6 tests fail
The record kept in memory and never committed	3 fail, including the restart one

One thing that might be worth adding to the case for this, from the leanVM side. Since 48a90420, aggregation groups claims by (epoch, message) rather than by epoch, so an aggregate will now carry two different messages signed by one key at one slot. I tried the same input on both commits while writing #1209 and the older one rejected it with ConflictingMessages. So the aggregation layer is no longer catching key reuse as a side effect, and this PR is what is left. If anything I think this matters more now than when you opened it.

Two small things I noticed, both take-them-or-leave-them.

The storage call can move a record backwards. put_last_signed_slot is INSERT OR REPLACE, so a lower slot overwrites a higher one. I checked: writing 10 and then 3 leaves it at 3. Nothing does that today, since reserve checks first, so this is belt and braces rather than a bug. But as that one number is the whole point of the table, it might be nice to let SQLite hold the invariant too:

INSERT INTO signing_records (validator_index, key_role, last_signed_slot)
VALUES (?, ?, ?)
ON CONFLICT (validator_index, key_role) DO UPDATE SET
  last_signed_slot = MAX(last_signed_slot, excluded.last_signed_slot)

I ran that against your table definition to be sure it behaves: 10, then 3, then 12 leaves 10, 10, 12. ON CONFLICT needs SQLite 3.24 and the project is on Python 3.12, so that is comfortably covered.

The durability is real, but it is inherited. I was curious what the connection actually runs at, and it is synchronous = 2 with journal_mode = delete, so a commit does reach the disk and the guard survives a power cut. The only snag is that nothing says the guard depends on that, and switching to WAL or synchronous = NORMAL is usually the first thing anyone tries when they want the database quicker. A sentence in the docstring might save a future reader from trading it away without noticing.

And a question, which may well be a non-issue. Records are keyed by validator index, while the thing that must not be reused is the key. The manifest lists public_key_hex next to each index, so anyone who reassigns indices onto existing key files would get a clean record for a spent key. I have no idea whether that happens in practice on these devnets, and you would know far better than I would.

None of this stops it merging as far as I am concerned. Happy to send the SQL change as a small PR against your branch if that is useful, or to leave it entirely with you.