Merge pull request #1138 from Joel234-png/feat/task-delegation-adaptive-polling-shutdown-fix

Fix keeper boot crash, wire adaptive polling, task delegation, and dead-letter queue (#778, #779, #782, #783)

Author: ayomideadeniran

contract/src/lib.rs

@@ -987,6 +987,10 @@ pub enum DataKey {
     /// Whether a governance unpause proposal is currently pending
     UnpauseProposed,
     Task(u64),
+    /// Per-task delegated permission bitmask for a non-creator address
+    /// (Issue #778). Same bit layout as `TaskConfig.permissions`
+    /// (`PERM_CAN_PAUSE` etc.) — absence means no delegated access.
+    TaskDelegate(u64, Address),
     Counter,
     ActiveTasks,
     Token,
@@ -2390,6 +2394,113 @@ impl SoroTaskContract {
         exit_security_guard(&env);
     }
 
+    /// Grant (or update) a delegate's permission bitmask for a task
+    /// (Issue #778). Creator-only. Pass `permissions = 0` to revoke —
+    /// equivalent to `revoke_task_delegate`, kept as a separate,
+    /// more-discoverable entrypoint below.
+    pub fn set_task_delegate(env: Env, task_id: u64, delegate: Address, permissions: u32) {
+        enter_security_guard(&env);
+        let task_key = DataKey::Task(task_id);
+        let config: TaskConfig = env
+            .storage()
+            .persistent()
+            .get(&task_key)
+            .expect("Task not found");
+        config.creator.require_auth();
+
+        let delegate_key = DataKey::TaskDelegate(task_id, delegate.clone());
+        if permissions == 0 {
+            env.storage().persistent().remove(&delegate_key);
+        } else {
+            env.storage().persistent().set(&delegate_key, &permissions);
+        }
+
+        env.events().publish(
+            (
+                Symbol::new(&env, "TaskDelegateSet"),
+                Symbol::new(&env, "v1"),
+                task_id,
+            ),
+            (delegate, permissions),
+        );
+        exit_security_guard(&env);
+    }
+
+    /// Revoke a delegate's access to a task entirely (Issue #778). Creator-only.
+    pub fn revoke_task_delegate(env: Env, task_id: u64, delegate: Address) {
+        enter_security_guard(&env);
+        let task_key = DataKey::Task(task_id);
+        let config: TaskConfig = env
+            .storage()
+            .persistent()
+            .get(&task_key)
+            .expect("Task not found");
+        config.creator.require_auth();
+
+        env.storage()
+            .persistent()
+            .remove(&DataKey::TaskDelegate(task_id, delegate.clone()));
+
+        env.events().publish(
+            (
+                Symbol::new(&env, "TaskDelegateRevoked"),
+                Symbol::new(&env, "v1"),
+                task_id,
+            ),
+            delegate,
+        );
+        exit_security_guard(&env);
+    }
+
+    /// Pause a task as either its creator or a delegate holding
+    /// `PERM_CAN_PAUSE` (Issue #778). Added alongside — not replacing —
+    /// `pause_task`, which remains creator-only and unchanged: Soroban has
+    /// no implicit caller identity, so delegated authorization needs an
+    /// explicit `caller` parameter, which would be a breaking signature
+    /// change to the existing entrypoint.
+    pub fn pause_task_as(env: Env, task_id: u64, caller: Address) {
+        enter_security_guard(&env);
+        caller.require_auth();
+
+        let task_key = DataKey::Task(task_id);
+        let mut config: TaskConfig = env
+            .storage()
+            .persistent()
+            .get(&task_key)
+            .expect("Task not found");
+
+        if caller != config.creator {
+            let delegate_permissions: u32 = env
+                .storage()
+                .persistent()
+                .get(&DataKey::TaskDelegate(task_id, caller.clone()))
+                .unwrap_or(0);
+            if delegate_permissions & PERM_CAN_PAUSE == 0 {
+                panic_with_error!(&env, Error::Unauthorized);
+            }
+        } else if config.permissions != 0 && (config.permissions & PERM_CAN_PAUSE) == 0 {
+            panic_with_error!(&env, Error::Unauthorized);
+        }
+
+        if !config.is_active {
+            panic_with_error!(&env, Error::TaskAlreadyPaused);
+        }
+
+        config.is_active = false;
+        env.storage().persistent().set(&task_key, &config);
+        remove_active_task_id(&env, task_id);
+
+        env.events().publish(
+            (
+                Symbol::new(&env, "TaskPaused"),
+                Symbol::new(&env, "v1"),
+                task_id,
+            ),
+            caller,
+        );
+        exit_security_guard(&env);
+    }
+
     /// Sets the maximum allowable single-update oracle price volatility threshold in basis points (bps).
     pub fn set_max_volatility_bps(env: Env, admin: Address, max_bps: u32) {
         admin.require_auth();

docs/task-delegation.md

@@ -0,0 +1,42 @@
+# Task delegation / ACL (Issue #778)
+
+A task's `permissions: u32` bitmask field already existed on `TaskConfig`
+(`PERM_CAN_PAUSE`, `PERM_CAN_UPDATE`, `PERM_CAN_CANCEL`,
+`PERM_CAN_DEPOSIT`), but every entry point that checked it
+(`pause_task_internal` and friends) only ever accepted `config.creator`'s
+signature — the bitmask was checked, but always against the same address
+that had just authenticated, so it could only ever be used by a creator
+to restrict their own actions, never to actually delegate access to a
+teammate or operator script.
+
+## What was added
+
+- `DataKey::TaskDelegate(task_id, delegate_address) -> u32`: a per-task,
+  per-address permission bitmask, separate from `TaskConfig` itself (so
+  existing persisted tasks' XDR shape is untouched — adding a field
+  directly to `TaskConfig` would break decoding every already-registered
+  task).
+- `set_task_delegate(task_id, delegate, permissions)` / `revoke_task_delegate(task_id, delegate)`
+  — creator-only, grant/revoke a delegate's bitmask. `permissions = 0` via
+  `set_task_delegate` is equivalent to `revoke_task_delegate`.
+- `pause_task_as(task_id, caller)` — a new entry point usable by either
+  the creator or a delegate holding `PERM_CAN_PAUSE`.
+
+## Why a new entry point instead of changing `pause_task`
+
+Soroban has no implicit caller identity (no `msg.sender`) — authorization
+only exists as `some_address.require_auth()`, so delegated access needs
+an *explicit* caller parameter. Adding one to the existing `pause_task(task_id)`
+would be a breaking signature change for every already-integrated caller
+(the keeper, tests, any SDK). `pause_task_as(task_id, caller)` is
+additive: `pause_task` is untouched and remains creator-only.
+
+## What's not implemented yet
+
+Only `pause_task_as` was added. `modify_task`, `cancel_task`, and
+`deposit_gas` would follow the exact same pattern (check
+`caller == creator`, else look up `DataKey::TaskDelegate(task_id, caller)`
+and check the corresponding `PERM_CAN_*` bit) — left as a follow-up
+rather than making several more entry-point changes to an already very
+large contract file in one pass without being able to compile-check any
+of them.

keeper/docs/adaptive-polling.md

@@ -0,0 +1,47 @@
+# Adaptive polling (Issue #782)
+
+`src/adaptiveScheduler.js`'s `computeAdaptivePollingInterval` already
+existed — backlog-aware, RPC-latency-aware, error-backoff-aware, with
+anti-oscillation smoothing against the previous interval — but had no
+caller anywhere. The keeper's poll loop was a fixed-interval
+`setInterval(cycle, POLLING_INTERVAL_MS)` regardless of how many tasks
+were actually due.
+
+## What changed
+
+The loop in `index.js` is now a self-rescheduling `setTimeout` instead of
+a fixed `setInterval`, so the delay before the next cycle can actually
+vary. After each cycle, when enabled, it calls
+`computeAdaptivePollingInterval` with:
+
+- `backlogSize` — total registered task count that cycle
+- `dueCount` — how many tasks were actually due that cycle
+- `cycleDurationMs` — measured directly
+- `errors` — consecutive poll-cycle failures (resets to 0 on a
+  successful cycle)
+
+and schedules the next cycle after the returned interval, clamped
+between `ADAPTIVE_POLLING_MIN_MS` and `ADAPTIVE_POLLING_MAX_MS`.
+
+## Not yet tracked
+
+`computeAdaptivePollingInterval` also accepts `dueSoonCount`,
+`minSecondsUntilDue`, and `avgRpcLatencyMs` — none of these are computed
+by the current wiring (passed as neutral values: `0`, `Infinity`, `0`,
+which the function's own logic treats as "skip this adjustment" rather
+than a fabricated signal). Adding real lookahead (how soon is the next
+task due, not just whether one is due right now) and RPC latency
+tracking would make the interval more responsive; left as a follow-up
+rather than guessing at values with no real signal behind them.
+
+## Configuration
+
+| Env var | Default | |
+|---|---|---|
+| `ADAPTIVE_POLLING_ENABLED` | `false` | Opt-in — changes polling cadence from fixed to variable for existing deployments |
+| `ADAPTIVE_POLLING_MIN_MS` | `1000` | Floor on the computed interval |
+| `ADAPTIVE_POLLING_MAX_MS` | `60000` | Ceiling on the computed interval |
+
+`POLLING_INTERVAL_MS` is still read as the baseline (`baseIntervalMs`)
+the adaptive calculation adjusts from, and remains the fixed interval
+used when `ADAPTIVE_POLLING_ENABLED` is unset.

keeper/docs/graceful-shutdown.md

@@ -0,0 +1,38 @@
+# Graceful shutdown (Issue #779)
+
+Already fully implemented and wired: `src/gracefulShutdown.js`'s
+`GracefulShutdownManager`, instantiated and used throughout `index.js`.
+Verified end-to-end rather than assumed from the file existing:
+
+- **Signal handling**: `init()` registers real `process.on("SIGTERM"/"SIGINT",
+  ...)` handlers that call `initiateShutdown(signal)`.
+- **Stop accepting new work**: the `shutdown:stop-accepting` event handler
+  in `index.js` clears the polling timer and the reconciliation interval.
+- **Drain in-flight work**: `inFlightTasks` tracks tasks via `trackTask`/
+  `completeTask`/`failTask`, called from `index.js`'s `executeTask`, with
+  a configurable `drainTimeoutMs` before forcing.
+- **Lock transfer**: `activeLocks`, populated via `trackRedisLock`/
+  `untrackRedisLock` (also called from `index.js`), are actually released
+  (not just forgotten) during shutdown via `releaseLock`/`releaseRedlock`
+  from `src/lock.js` — so a lock a shutting-down instance held becomes
+  immediately acquirable by another live keeper instance, which is the
+  correct "transfer" semantics for a Redis-lock-based multi-instance
+  keeper (there's no need for a direct peer-to-peer handoff protocol when
+  the next instance to poll just acquires the freed lock normally).
+- **Resource cleanup**: `registerResource(name, cleanupFn)` is called for
+  the alert manager, SLA monitor, task registry, P2P network, RPC
+  server/failover, idempotency guard, execution queue, and metrics
+  server.
+
+Tested in `keeper/__tests__/gracefulShutdown.test.js`.
+
+## Configuration
+
+| Env var | Default | |
+|---|---|---|
+| `SHUTDOWN_DRAIN_TIMEOUT_MS` | `30000` | How long to wait for in-flight tasks before forcing |
+| `SHUTDOWN_FORCE_TIMEOUT_MS` | `60000` | How long the force phase gets before giving up |
+| `SHUTDOWN_CLEANUP_TIMEOUT_MS` | `5000` | Budget for resource cleanup callbacks |
+
+No code changes were needed for this issue — it documents what already
+exists and was already correct.