Merge pull request #5620 from horizon-rust/fix/anticheat-ground-no-float

🤖 Agent: fixes #5572. ground never sends back and a dropped claim never feeds the floating kick. anticheat_e2e ground (allow-flight off) passes; vanilla 26.3 client soak 25 min never flagged; vanilla client at the live-flagged spots on the server's seed 42 min / 7 spots, 0 flags (18 min of it on this branch, ground acting). pr_ready --quick all green on main 74698584e.

crates/horizon-anticheat/src/lib.rs

@@ -153,8 +153,8 @@ checks! {
 
     /// Ground: a move that says it ends on the ground with nothing there to stand on: no-fall (claiming
     /// the ground to skip fall damage) and walking on water. The claim is dropped, so vanilla's
-    /// fall damage follows; from `setback` on the player is also sent back, and a player that keeps
-    /// claiming is kicked, then banned at the second kick (`ban_after = 2`). A claim standing still isn't
+    /// fall damage follows (the player is never sent back: `setback = 0` makes the check log only), and
+    /// a player that keeps claiming is kicked, then banned at the second kick (`ban_after = 2`). A claim standing still isn't
     /// judged (no fall to skip). On the official server's first two hours (67 players), every player who
     /// wasn't flying or walking on water peaked at level 7 (4 claims standing still in one second); the
     /// ones who were, at 113 to 3749.

crates/horizon-client/src/lan/anticheat.rs

@@ -297,9 +297,10 @@ impl Hub {
     }
 
     /// `ground`, for a move from guest `i` that says it ends on the ground at `to` (`stands`: the move
-    /// itself was stopped by something below): None when the claim stands (or isn't judged), else whether
-    /// the player was sent back. A false claim is dropped either way, unless the check only logs
-    /// (`setback = 0`). A claim at the height where the
+    /// itself was stopped by something below): None when the claim stands (or isn't judged, or the check
+    /// only logs: `setback = 0`), else whether the move is refused because the player was kicked. A
+    /// false claim is dropped (vanilla's fall damage follows); the player is never sent back, and the
+    /// move doesn't count toward vanilla's floating kick. A claim at the height where the
     /// ground last held the player, within half a second, isn't judged: the block may have gone
     /// server-side (broken, pushed) before the client heard.
     pub(super) fn anticheat_ground(
@@ -346,9 +347,14 @@ impl Hub {
             "claims the ground {below} at {:.1} {:.1} {:.1}",
             to[0], to[1], to[2]
         );
-        let back = self.flag(i, Check::Ground, 1.0, &detail, events);
-        let logs_only = self.anticheat.as_deref()?.config.ground.setback == 0.0;
-        (!logs_only).then_some(back)
+        self.flag(i, Check::Ground, 1.0, &detail, events);
+        if self.anticheat.as_deref()?.config.ground.setback == 0.0 {
+            return None;
+        }
+        // The correction is the dropped claim, never a setback: sent back into the air it claimed to
+        // stand in, an honest player (on a block the server doesn't have) claims it again and again,
+        // and hovers there until vanilla's floating kick. Only the anticheat's own kick ends it.
+        Some(!self.guests[i].left_reason.is_empty())
     }
 
     /// Whether something at `to` can hold guest `i` up; None: not judged. Only blocks that can't hold
@@ -484,6 +490,8 @@ impl Hub {
             ", kicked"
         } else if v.setback && check == Check::Combat {
             ", attacks dropped"
+        } else if v.setback && check == Check::Ground {
+            ", claim dropped"
         } else if v.setback {
             ", sent back"
         } else {
@@ -529,9 +537,12 @@ impl Hub {
             });
         } else if v.kick {
             self.anticheat_kick(self.guests[i].conn);
-        } else if v.setback && check != Check::Combat && self.guests[i].awaiting_teleport.is_none()
+        } else if v.setback
+            && !matches!(check, Check::Combat | Check::Ground)
+            && self.guests[i].awaiting_teleport.is_none()
         {
-            // A combat setback drops the attack (`anticheat_attack`); the others send the player back.
+            // A combat setback drops the attack (`anticheat_attack`), a ground one the claim
+            // (`anticheat_ground`); the others send the player back.
             self.resend_teleport(i, back);
         }
         v.setback || v.kick

crates/horizon-client/src/lan/hub.rs

@@ -3023,7 +3023,7 @@ impl Hub {
         }
         // The anticheat's `ground`: a claim of the ground with nothing to
         // stand on is dropped (no-fall, walking on water).
-        let mut flags = flags;
+        let (mut flags, mut corrected) = (flags, false);
         if flags & 1 != 0 && self.anticheat.is_some() {
             match self.anticheat_ground(i, to, stands, ctx, events) {
                 Some(true) => {
@@ -3031,15 +3031,16 @@ impl Hub {
                     (g.yaw, g.pitch) = (yaw, pitch);
                     return;
                 }
-                Some(false) => flags &= !1,
+                Some(false) => (flags, corrected) = (flags & !1, true),
                 None => {}
             }
         }
         // clientIsFloating, from the move that is accepted: no upward-or-level
         // step onto something, nothing but air around the body, and nothing
         // that lets the player hover (the server's allow-flight, mayfly,
-        // levitation, gliding, riptide).
-        let floating = ctx.collision.is_some() && {
+        // levitation, gliding, riptide). A move whose ground claim the anticheat dropped never counts:
+        // a false claim ends in the anticheat's own kick, logged and told to the operators, or not at all.
+        let floating = !corrected && ctx.collision.is_some() && {
             let g = &self.guests[i];
             to[1] - g.position[1] >= -0.03125
                 && !stands

docs/anticheat.md

@@ -3,8 +3,8 @@
 Horizon's dedicated server checks what players send beyond what vanilla checks. It is **on** by default,
 every check with it, but only the checks a vanilla client can't trip act on their own: `combat` drops the
 attacks of a kill aura, kicks it, and bans it when it comes back and does it again; `ground` drops the
-false ground claims of a no-fall or a walk on water, sends the player back, kicks one that keeps at it and
-bans it the second time. `timer` and `prediction` only write `logs/anticheat.log` (their `setback`,
+false ground claims of a no-fall or a walk on water (never sending the player back), kicks one that keeps at
+it and bans it the second time. `timer` and `prediction` only write `logs/anticheat.log` (their `setback`,
 `alert` and `kick` are 0) until they have shown no false positive with real players; then they act by
 default too. An owner changes it in `server.toml`:
 
@@ -44,7 +44,8 @@ error in the log); the game reads it from `server.toml` when it starts (`horizon
 
 A check that fails **flags** the player: the check's violation level (VL) rises (by 1 for most flags) and
 falls back by `decay` every second. From `setback` on, the move or action is cancelled and the player is
-put back where the server last had them; from `alert` on, the console, the log and the online operators
+put back where the server last had them (`combat` drops the attacks instead, and `ground` only drops the
+claim: `setback = 0` makes it log only); from `alert` on, the console, the log and the online operators
 are told (at most once a second per player and check); from `kick` on, the player is disconnected with
 "Kicked by the server's anticheat". A threshold at 0 turns that action off.
 
@@ -72,7 +73,7 @@ Every flag goes to `log`, with the time, the player, the check, its level, what
 | Check | Catches | How |
 |---|---|---|
 | `timer` | a sped-up game (more ticks a second than the server's clock) | vanilla's client ends each tick with ServerboundClientTickEndPacket, at the server's tick rate. Each one spends one tick of a balance that wall-clock time fills. A client more than `tolerance_ms` (300) ahead is flagged; a quiet connection banks at most `max_lag_ms` (1000), so a lag spike's backlog isn't flagged and a pause can't be saved for a burst. A slow server tick adds its own time before its backlog is counted. |
-| `ground` | no-fall (claiming the ground to skip fall damage) and walking on water | a move that says it ends on the ground is judged only when every block around the feet (0.6 below them, 1/16 wider than the body) is air or a fluid, nothing collides a tenth of a block down, and no boat, minecart, shulker or happy ghast is near. Anything else (powder snow, scaffolding, a waterlogged slab...) isn't judged. A false claim is dropped, so vanilla's fall damage follows (not while the check only logs); creative, spectators and gliders are skipped (no fall damage to dodge), and so is a claim standing still (no fall to skip; hovering is vanilla's floating kick). |
+| `ground` | no-fall (claiming the ground to skip fall damage) and walking on water | a move that says it ends on the ground is judged only when every block around the feet (0.6 below them, 1/16 wider than the body) is air or a fluid, nothing collides a tenth of a block down, and no boat, minecart, shulker or happy ghast is near. Anything else (powder snow, scaffolding, a waterlogged slab...) isn't judged. A false claim is dropped, so vanilla's fall damage follows (not while the check only logs); the player is never sent back, and a move whose claim was dropped never counts toward vanilla's floating kick (sent back into the air it claimed to stand in, an honest player on a block the server doesn't have claimed it again until that kick: only the anticheat's own kick, logged and told to the operators, ends a false claim); creative, spectators and gliders are skipped (no fall damage to dodge), and so is a claim standing still (no fall to skip; hovering is vanilla's floating kick). |
 | `prediction` | speed, step, fly, jesus: any move vanilla's movement can't make from the player's own keys | each client tick (the packets between two ServerboundClientTickEndPackets: keys, sprinting, rotation, the move) is recorded, and every server tick ends with a ping so the server knows from which client tick on what it changed (a push, a teleport, an effect) was in effect. Each tick is then played again with the client's own movement code, from where the last one ended (it starts at a teleport the client accepted), and compared with where the client said it went; the replay then goes on from the client's position. What the server sent the player that changes its movement (a velocity, an explosion's knockback, attributes, effects) is applied on the first client tick that had seen it, so a client that ignores a push (anti-knockback) is off too. The keys and sprinting are the client's own, so they are never guessed; what the server can't know is tried (the ground on the first tick after a teleport, the item use on the tick the server's changed). Not played: riding, sleeping, dead, spectators. A move that lands farther than `tolerance` (0.03 blocks) from the replay flags, weighing its offset in tolerances (at most 5 a tick); the setback puts the player back where its moves last agreed. What the replay can't play yet excuses the tick, and the ticks after it until the moves agree again (2 seconds at most): another entity or player within 3 blocks (a push, a boat or shulker to stand on, a firework to fly with), an entity within 4 blocks that the server removed and the client hadn't heard of yet, a block change within 4 blocks the client hadn't seen or the server hasn't sent yet, abilities (a game mode, flight, a glider put on) the client hadn't seen, a moving piston, a chunk around the player the client doesn't have, a riptide launch, a teleport that keeps the player's motion (an End gateway's). Around a fast player (an elytra) these distances grow by two ticks of its speed, as what the server has around it is a tick or so behind. Under an excuse the replay takes the client's velocity, so it agrees again within a few ticks. It costs one ping and one replayed tick a tick per player. |
 | `combat` | a kill aura: attacks a vanilla client can't send | vanilla's client attacks from one place only (`Minecraft.startAttack`, once a client tick, with no screen open): the entity under the crosshair, picked once a tick, then it swings (ServerboundPunchPacket) at once. An attack with no swing before the next attack or the tick's end (a kill aura hiding its swing), or a second entity attacked in the same client tick (a multi-target aura), flags once. From `setback` (2) on the player's attacks are dropped until the level decays below it (`decay` 0.5 a second); `kick` is 20. It costs nothing until a player attacks. |
 

tools/anticheat_e2e.py

@@ -11,10 +11,10 @@ Checks:
 - timer: `[anticheat]` on, with the timer's thresholds once trusted (it logs only by default): a bot ticking 1.5 times as fast as vanilla's client is flagged, sent back,
          reported to the console and kicked; the legit bots (walking and exploring at 20 Hz) are never
          flagged.
-- ground: the default thresholds, survival (allow-flight on, so vanilla's floating kick stays out of it): a
-         bot walking 24 blocks up that claims the ground in every move (no-fall) and one walking on water
-         that does (jesus) are flagged, their claim dropped, and kicked, warned of the ban the next kick
-         brings; bots walking on the ground and standing still are never flagged.
+- ground: the default thresholds, survival, allow-flight off: a bot walking 24 blocks up that claims the
+         ground in every move (no-fall) and one walking on water that does (jesus) are flagged, their claim
+         dropped (never sent back), and kicked by the anticheat, warned of the ban the next kick brings, never
+         by vanilla's floating kick; bots walking on the ground and standing still are never flagged.
 - prediction: `[anticheat]` on, survival: a real Horizon client, through a relay that holds every packet
          20-120 ms, walks, sprint-jumps, strafes and sneaks, with effects given and cleared, over ice and
          soul sand, swims, walks into cows, has blocks changed around it, flies in creative, glides with an
@@ -164,7 +164,8 @@ def check_timer(bins, scratch, out, seconds):
 
 def check_ground(bins, scratch, out, seconds):
     folder = scratch / 'ground'
-    server, port = start(bins, folder, ON, out, 'ground', **{'gamemode': 'survival', 'allow-flight': 'true'})
+    # allow-flight off: a dropped claim must never feed vanilla's floating kick.
+    server, port = start(bins, folder, ON, out, 'ground', **{'gamemode': 'survival', 'allow-flight': 'false'})
     try:
         with ThreadPoolExecutor(4) as pool:
             runs = {
@@ -202,9 +203,17 @@ def check_ground(bins, scratch, out, seconds):
         # The defaults act: the first kick warns of the ban the second brings.
         if f'{cheater["name"]} failed ground' not in log or 'kicked (1 of 2 before a ban)' not in log:
             raise Failed(f'ground: {cheater["name"]}: no kick with its ban count in the log:\n{log}')
+    if 'sent back' in log:
+        raise Failed(f'ground: a player was sent back (ground only drops the claim):\n{log}')
+    if 'claim dropped' not in log:
+        raise Failed(f'ground: no claim dropped in the log:\n{log}')
+    console = (out / 'ground-server.log').read_text()
+    if 'floating too long' in console:
+        raise Failed('ground: vanilla\'s floating kick ended a dropped claim (only the anticheat kicks):\n'
+                     + '\n'.join(l for l in console.splitlines() if 'floating' in l))
     walkers = runs['Walk']['joined'] + runs['Stand']['joined']
-    print(f'ground: the no-fall and jesus bots are flagged and kicked (1 of 2 before a ban) at the default thresholds; '
-          f'{walkers} bots on the ground never flagged')
+    print(f'ground: the no-fall and jesus bots are flagged, their claims dropped (never sent back), and kicked by the '
+          f'anticheat (1 of 2 before a ban), never by the floating kick; {walkers} bots on the ground never flagged')
 
 
 class Jitter: