fix(shade-contract-template): refund register deposit above the storage cost

[PiVortex]

Closes #64

What & why

register_agent previously required attached_deposit >= storage_cost for first-time registrations and kept any overpayment; re-registrations were unchecked. This reworks it to:

• compute required_deposit = the storage cost for a new agent, 0 for a re-registration (reuses the existing slot);
• require attached >= required_deposit;
• refund the excess (attached − required) to the agent.

So overpayment can no longer be lost, and a caller can safely attach more than the minimum — which removes the brittle exact-match coupling between the client default and the contract (the issue that dogged the earlier exact-deposit attempt).

Failure handling

A failed registration panics — insufficient deposit (require!) or an invalid attestation (verify_attestation). On panic NEAR reverts the whole tx and auto-refunds the full attached deposit, so there's no manual refund on the failure path and no need for a non-panic path. The explicit Promise::transfer refund runs only on success, for the overpaid excess. The error surfaces via the panic message → register() → call() → account.callFunction() rejects → toThrowable rethrows it, so the agent gets a clear, specific error.

Files changed

• shade-contract-template/src/lib.rs — register_agent reworked (required_deposit + >= + refund excess via Promise::transfer(..).detach()); kept STORAGE_BYTES_TO_REGISTER with a short fork/mirror comment.
• shade-agent-js/src/api.ts — DEFAULT_REGISTER_DEPOSIT_YOCTO → 4860000000000000000000 (0.00486 NEAR, the storage cost); JSDoc notes a higher deposit is allowed (refunded) and that forceDeposit: true is now safe on re-registration.
• docs/reference/agent-contract.md, docs/reference/api.md — updated for >= + refund, the re-registration-needs-0 rule, and "you may attach more (refunded)".
• shade-agent-js/tests/unit/api.test.ts — default-deposit fixture → 0.00486.
• shade-contract-template/src/internal/unit_tests.rs — added refund tests (overpay → 0.00014 refund; exact → no refund; re-register → full refund); updated the insufficient-deposit panic-message expectation.

Tests

• Contract cargo test --lib (54 pass, incl. 3 new refund tests); cargo clippy --all-targets clean; integration tests still compile (cargo test --no-run) and need no deposit changes since 0.005 ≥ 0.00486.
• shade-agent-js build + 275 unit tests; shade-agent-template tsc clean.

Design decisions / Accepted tradeoffs

• >= + refund chosen over exact-== (avoids the client↔contract coupling / version-skew the reviews flagged) and over runtime-measured storage (kept the constant for simplicity, per maintainer decision).
• Re-register-after-removal still re-charges storage (issue Fix agent deposits #64 point 2): removal doesn't refund the deposit. Accepted and documented on remove_agent.

Release impact

@neardefi/shade-agent-js — minor. The default register deposit changed (0.005 → 0.00486); backward-compatible (the contract refunds excess, so any over-attach is safe). Version bump handled on main per the release process.

Follow-up

• tests-in-tee (/run-e2e) must be run by a maintainer — the only end-to-end proof of the registration flow under attestation. Its test-different-account-id scenario needs no change (0.005 still ≥ the cost).
• Supersedes fix(shade-contract-template): require exact register deposit with no refund #95 (the exact-deposit approach), which is being closed.

🤖 Generated with Claude Code

[PiVortex]

/claude-review

[Copilot]

Pull request overview

Updates the Shade agent registration flow to avoid silently keeping excess attached deposits: the contract now charges only the required storage stake (or 0 on re-registration) and refunds any overpayment, while the JS client/docs/tests are updated to reflect the new default and semantics.

Changes:

• Reworks register_agent to compute a required_deposit (storage cost for new agents, 0 for re-registrations), require attached >= required, and refund attached - required on success.
• Updates shade-agent-js default register deposit to the current storage cost (0.00486 NEAR) and clarifies that attaching more is safe (excess refunded).
• Updates docs and adds/updates unit tests to cover refund behavior and the new error message wording.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
shade-contract-template/src/lib.rs	Implements required_deposit logic and refunds any excess deposit after successful registration.
shade-contract-template/src/internal/unit_tests.rs	Adds unit tests covering overpay refund, exact-deposit no-refund, and re-register full refund; updates panic expectations.
shade-agent-js/src/api.ts	Updates DEFAULT_REGISTER_DEPOSIT_YOCTO to 0.00486 NEAR and updates JSDoc to document refund-safe over-attachment.
shade-agent-js/tests/unit/api.test.ts	Updates the expected default deposit fixture and test wording to match the new default.
docs/reference/api.md	Updates register() documentation to describe “>= + refund” semantics and the new default deposit.
docs/reference/agent-contract.md	Updates contract documentation and example snippet to describe required-deposit and refund behavior, including re-registration requiring 0.

[claude]

Code review

Reviewed the register_agent rework (>= required_deposit + refund-the-excess, required_deposit = 0 on re-registration) and the shade-agent-js default-deposit change (0.005 → 0.00486 NEAR). The contract logic is correct and safe: the checked_sub(...).unwrap() at the refund site can't underflow given the preceding require!(attached >= required_deposit); refund <= attached always holds, so the contract can never pay out more than was just attached (no drain); the detached transfer is a bare native-token transfer scheduled after all state mutation, so there's no re-entrancy; and required_deposit = 0 on re-register is sound because Agent is entirely fixed-size fields (no new storage). Attestation gating, owner access control, signing, and secret handling are untouched. Docs (agent-contract.md, api.md), the JS default, and the new Rust/TS tests are all numerically consistent (486 × 1e19 = 4.86e21 yocto = 0.00486 NEAR), and no version was bumped.

Found 1 issue:

1. [MEDIUM:55] No tests-in-tee/ scenario exercises the new on-chain refund — the core behavior of this PR. The root CLAUDE.md lists "Agent registration flow" as a trigger for updating tests-in-tee/ scenarios. The new unit tests cover the refund only via mocked receipts (get_created_receipts confirms the Promise is scheduled, not that the transfer lands), and the existing e2e successful-registration scenario now attaches exactly the storage cost (the no-refund path) and asserts only isAgentRegistered — nothing over-deposits and asserts the excess is refunded to the agent account under real attestation. The refund is a plain NEAR transfer (attestation-independent), so the gap is low-risk, but per CLAUDE.md an over-deposit → refund assertion is the kind of registration-flow change tests-in-tee/ is meant to cover.

shade-agent-framework/shade-contract-template/src/lib.rs

Lines 137 to 140 in 53467a3

	let refund = attached.checked_sub(required_deposit).unwrap();
	if refund > NearToken::from_yoctonear(0) {
	Promise::new(predecessor).transfer(refund).detach();
	}

Design notes

• The refund uses Promise::new(predecessor).transfer(refund).detach(), so a refund-transfer failure surfaces nowhere and wouldn't revert the registration. This is the standard/correct NEAR refund pattern and the predecessor account provably exists (it just signed the tx), so it's acceptable — noted only for completeness.
• Existing tests-in-tee/ and contract integration scenarios still attach 0.005 NEAR for first-time registration. This is fine and acknowledged in the PR (0.005 ≥ 0.00486; the excess is now refunded), and the PR notes tests-in-tee must be run by a maintainer.
• The PR's accepted tradeoffs — >= + refund over exact-==/runtime-measured storage, and re-register-after-removal still re-charging storage — were not re-evaluated, per the review instructions.

[PiVortex]

Addressed in ce27db7 — added test_register_agent_refunds_excess_deposit_integration (sandbox integration test, runs in /run-e2e): it over-deposits 1 NEAR on registration and asserts the agent's on-chain balance drops by only ~storage + gas (< 0.1 NEAR), proving the excess is actually refunded (not just that the receipt is scheduled). Put it in the sandbox suite rather than tests-in-tee because the refund is a plain native transfer (attestation-independent), so a TEE run adds nothing — and this one is actually runnable in CI's e2e gate. (Unit tests still assert the exact refund amount via the scheduled receipt.)

[PiVortex]

/claude-review

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

[claude]

Code review

Found 1 issue:

1. [LOW:60] shade-contract-template/README.md test inventory not updated for this PR's new tests. The Integration tests table lists both sibling test_register_agent_*_integration tests but omits the new test_register_agent_refunds_excess_deposit_integration; likewise the Unit tests prose ("…insufficient / missing deposit on first registration") doesn't mention the three new refund unit tests (test_register_agent_refunds_overpayment, test_register_agent_exact_deposit_no_refund, test_register_agent_reregister_refunds_full_deposit). Root CLAUDE.md requires docs to track package changes, and this inventory is clearly maintained per-test.

shade-agent-framework/shade-contract-template/README.md

Lines 56 to 68 in ce27db7

	### Integration tests
	| Test | What it tests |
	| ---------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
	| `test_measurements_and_ppid_lifecycle` | Exercises the full measurements and PPID approval lifecycle. Verifies that `request_signature` fails with InvalidMeasurements, InvalidPpid, or both when approvals are revoked; that the agent is removed and the right events are emitted; that re-approving measurements and PPID restores access; and that a removed agent cannot re-register. |
	| `test_register_fails_without_default_measurements_or_ppid` | Confirms that registration fails in local mode when the default measurements or the default PPID have not been approved by the owner. |
	| `test_attestation_expiration` | Fast-forwards the sandbox past the attestation expiration time, then checks that `request_signature` removes the agent with ExpiredAttestation, that the next call returns "Agent not registered", and that the agent can re-register afterward. |
	| `test_cross_contract_call_to_mpc` | Ensures that `request_signature` correctly calls the mock MPC contract for both Ecdsa and Eddsa, and that updating `mpc_contract_id` causes later calls to use the new contract. |
	| `test_large_dataset_pagination_real_contract` | Registers 20 agents and checks that `get_agents` pagination works as expected using `from_index` and `limit`. |
	| `test_owner_transfer_and_new_owner_operations` | Transfers contract ownership and verifies that the new owner can approve measurements while the old owner can no longer do so. |
	| `test_update_contract` | Deploys the contract, calls `update_contract` with new WASM, and checks that state is migrated correctly and that the new methods are available. |
	| `test_register_agent_new_agent_requires_storage_deposit_integration` | First `register_agent` with no attached deposit fails; with `0.005 NEAR` succeeds; `get_agent` shows a valid agent. |
	| `test_register_agent_reregister_without_storage_deposit_integration` | After a successful first registration, `register_agent` again with no deposit succeeds.|

Otherwise clean: attestation gating, owner access control, signing, and secret handling are untouched; verify_attestation still runs before any state mutation; the refund is safe (checked_sub guarded by the preceding require!, so no underflow; refund ≤ attached, so no drain; detached transfer scheduled after all state writes). The storage-cost number is consistent across every touched artefact (486 × 1e19 = 4.86e21 yocto = 0.00486 NEAR in lib.rs, the unit tests, api.ts, api.test.ts, and both docs), shade-agent-template needs no change (it calls register() with no args), and no version was bumped.

Design notes

• Buffer removal (by design). With >= + refund the contract now retains exactly its 486-byte estimate and refunds the rest, rather than incidentally over-collecting the old 0.005-vs-0.00486 surplus. A fork that stores more per agent — or relies on long account IDs pushing real IterableMap storage past 486 bytes — without bumping STORAGE_BYTES_TO_REGISTER would have the contract subsidize the shortfall from its own balance (the tx still succeeds; it can't underpay the caller). The constant is kept deliberately per the maintainer decision and the lib.rs comment already flags it — noted for completeness.
• Already settled, not re-raised: the refund integration test placed in the sandbox suite rather than tests-in-tee/ (the refund is a plain, attestation-independent native transfer — adjudicated on the prior review round); the .detach() refund not surfacing transfer failures (standard NEAR pattern; predecessor provably exists); >= + refund chosen over exact-==/runtime-measured storage; and re-register-after-removal still re-charging storage.

[PiVortex]

Fixed in the latest push — added test_register_agent_refunds_excess_deposit_integration to the Integration tests table and the three refund unit tests (..._refunds_overpayment, ..._exact_deposit_no_refund, ..._reregister_refunds_full_deposit) to the Unit tests prose in shade-contract-template/README.md.

[PiVortex]

/claude-review

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

[claude]

Code review

No issues found.

Re-reviewed the current HEAD (4c248b9) after the two prior rounds were addressed — the refund integration test (test_register_agent_refunds_excess_deposit_integration) and the README.md test-inventory entries are both present. Four independent passes (security, architecture/cross-package sync, bug scan, performance) surfaced no new actionable findings.

Verified clean:

• Attestation & access control untouched. verify_attestation still runs before the only state mutation (agents.insert) and before the refund; register_agent stays payable and keyed on predecessor; owner-gated methods are unchanged. No new unauthenticated path to register, approve measurements/PPIDs, or sign.
• Refund safety. attached.checked_sub(required_deposit).unwrap() cannot underflow given the preceding require!(attached >= required_deposit); refund <= attached, so the contract can never pay out more than was attached; the detached transfer is the last action after all state writes (no re-entrancy). required_deposit = 0 on re-registration is sound — Agent is fixed-size, so re-registration allocates no new storage.
• Numeric consistency. 486 bytes x 1e19 yocto/byte = 4_860_000_000_000_000_000_000 yocto = 0.00486 NEAR is consistent across lib.rs, unit_tests.rs, api.ts, api.test.ts, agent-contract.md, api.md, and README.md.
• Docs & consumers in sync. agent-contract.md and api.md accurately describe the new behaviour (>= + refund, re-register needs 0, over-attach is safe); shade-agent-template calls register() with no args so it picks up the new default automatically; no package.json / Cargo.toml / lockfile versions were bumped.

Design notes

• The settled tradeoffs from earlier rounds were not re-evaluated: >= + refund chosen over exact-==/runtime-measured storage; the refund integration test placed in the sandbox suite rather than tests-in-tee/ (the refund is a plain, attestation-independent native transfer); the .detach() refund not surfacing transfer failures (standard NEAR pattern, predecessor provably exists); and re-register-after-removal still re-charging storage (issue Fix agent deposits #64 point 2, documented on remove_agent).

[PiVortex]

Reviews passed!

[PiVortex]

/run-e2e

[github-actions]

Running the e2e suite (contract integration + tests-in-tee): https://github.com/NearDeFi/shade-agent-framework/actions/runs/27932686818