fix(shade-contract-template): require exact register deposit with no refund

[PiVortex]

Closes #64

What & why

register_agent required attached_deposit >= storage_cost and kept any excess, so an agent that overpaid its storage deposit silently lost the difference (issue #64, point 1). This changes the check to require the deposit to match the storage cost exactly — no overpayment is possible, and the contract never has to refund:

• First-time registration must attach exactly STORAGE_BYTES_TO_REGISTER × storage_byte_cost = 0.00486 NEAR.
• Re-registration of an already-registered agent reuses the existing slot, so it must attach exactly 0.

Issue #64 point 2 (a removed agent re-registering pays the storage deposit again, since removal doesn't refund) is intentionally kept as-is for simplicity per discussion — now explicitly documented on remove_agent.

Because the contract now rejects any deposit other than the exact amount, shade-agent-js's default register deposit had to move from 0.005 → 0.00486 NEAR so register() keeps working.

Files changed

• shade-contract-template/src/lib.rs — register_agent now computes required_deposit (exact storage cost for new agents, 0 for re-registration) and requires attached_deposit == required_deposit; added a Contract::agent_storage_cost() helper as the single source of truth for the cost.
• shade-contract-template/src/internal/unit_tests.rs — test deposit constants reworked (EXACT_STORAGE_DEPOSIT, DEPOSIT_BELOW_COST, DEPOSIT_ABOVE_COST); registration setups now attach the exact amount; updated the two insufficient-deposit panic-message expectations.
• shade-agent-js/src/api.ts — DEFAULT_REGISTER_DEPOSIT_YOCTO → 4860000000000000000000, JSDoc updated.
• shade-agent-js/tests/unit/api.test.ts — default-deposit fixture updated to the exact amount.
• docs/reference/agent-contract.md — register_agent block + storage note rewritten for the exact-deposit/no-refund rule; remove_agent note now states re-registration pays again.
• docs/reference/api.md — register() default deposit, parameter description, example, and deposit-selection table updated.

Tests added/updated

New unit tests in shade-contract-template (run under cargo test --lib):

• test_register_agent_errors_when_storage_deposit_exceeds_cost — regression for Fix agent deposits #64: overpaying (0.005) is now rejected rather than silently kept.
• test_register_agent_errors_when_reregister_attaches_deposit — re-registration must attach exactly 0.
• test_agent_storage_cost_matches_expected — guards the hardcoded EXACT_STORAGE_DEPOSIT against storage-byte-cost drift.
• Existing zero/insufficient-deposit and happy-path tests updated to the exact-deposit semantics.

Verified locally: shade-contract-template cargo fmt / cargo clippy --all-targets (no new warnings) / cargo test --lib (54 pass); shade-agent-js npm run build + npm test (275 pass); shade-agent-template tsc clean.

Follow-up / maintainer notes

• tests-in-tee/ (/run-e2e) must be run by a maintainer — it exercises the live registration flow under attestation (the successful-registration scenario), which is the only place this change is proven end-to-end. The e2e image builds shade-agent-js from the local workspace, so the contract + library changes stay consistent there.
• Coupled release: the contract and shade-agent-js changes must ship together — an old published shade-agent-js (0.005 default) would fail registration against the new exact check. Version bump for @neardefi/shade-agent-js (currently 2.2.0) left to the release process.
• Brittleness note: the exact == check is sensitive to NEAR's per-byte storage price (today 1e19 yocto/byte). If that protocol parameter ever changes, the exact amount changes and clients must update; this is the trade-off accepted in choosing exact-deposit over a >=+refund design.

🤖 Generated with Claude Code

Release impact

@neardefi/shade-agent-js — minor. The default register deposit changed (0.005 → 0.00486 NEAR). Backward-compatible for library upgraders (0.00486 satisfies both the old >= contract and the new == contract), and it signals the coupled contract change. Version bump is handled on main per the release process (not in this PR).

[Copilot]

Pull request overview

This PR updates the agent registration flow in shade-contract-template to require an exact attached deposit (no overpayment/no refund) and aligns the JS client + reference docs with the new contract rule, addressing issue #64 (point 1).

Changes:

• Contract: register_agent now requires attached_deposit == required_deposit (exact storage cost for first-time registration, exactly 0 for re-registration).
• JS client: updates the default register deposit to 0.00486 NEAR (4860000000000000000000 yocto) so register() continues to work against the updated contract.
• Tests/docs: unit tests and reference docs updated to reflect the exact-deposit semantics and the “re-register attaches 0” rule.

Reviewed changes

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

Show a summary per file

File	Description
shade-contract-template/src/lib.rs	Enforces exact attached deposit and centralizes storage-cost calculation.
shade-contract-template/src/internal/unit_tests.rs	Updates registration deposit fixtures and adds regression tests for overpay / re-register deposit rules.
shade-agent-js/src/api.ts	Updates default register deposit constant + JSDoc to match contract behavior.
shade-agent-js/tests/unit/api.test.ts	Updates unit test fixture values to the new default deposit.
docs/reference/agent-contract.md	Updates contract reference to document and illustrate exact-deposit behavior.
docs/reference/api.md	Updates API docs for register() deposit defaults and behavior table.

[PiVortex]

Fixed in 6b4ae88. Reworded to "Attached deposit must be exactly {}" — accurate for re-registration too, where required = 0 (reads "…exactly 0 NEAR").

[PiVortex]

Fixed in 6b4ae88. The sandbox integration tests now use the exact deposit: tests/helpers/mod.rs DEPOSIT_005_NEAR → EXACT_STORAGE_DEPOSIT (0.00486 NEAR), with all helpers::-referencing calls updated, and README.md updated. Since the deposit check runs before verify_attestation, this matters for the measurement/PPID failure-path tests too. (Note: the sandbox suite isn't run in the PR gate — flagged for the maintainer e2e run.)

[PiVortex]

Fixed in 6b4ae88. Expectation updated to "Attached deposit must be exactly" to match the reworded message.

[PiVortex]

Fixed in 6b4ae88. Expectation updated to "Attached deposit must be exactly" to match the reworded message.

[PiVortex]

Fixed in 6b4ae88. Expectation updated to "Attached deposit must be exactly" to match the reworded message.

[PiVortex]

Fixed in 6b4ae88. Expectation updated to "Attached deposit must be exactly" to match the reworded message.

[PiVortex]

/claude-review

[PiVortex]

/claude-review

[claude]

Code review

Summarized: register_agent changes from attached_deposit >= storage_cost (keeping any overpayment) to attached_deposit == required_deposit — the exact storage cost (0.00486 NEAR) for a first-time registration and exactly 0 for re-registration — plus a new Contract::agent_storage_cost() helper, the shade-agent-js default register deposit moved 0.005 → 0.00486 NEAR, and docs/tests updated. The attestation-verification, owner-gating, and signing paths are unchanged; the deposit require! still runs before verify_attestation. The core changes look correct and well-tested.

Found 3 issues:

1. [HIGH:88] tests-in-tee was not updated for this registration-flow change (root CLAUDE.md requires it), and the change breaks Test 6. test-different-account-id.ts hardcodes deposit: NEAR.toUnits(0.005) for a raw register_agent call. Since the deposit require! runs before verify_attestation (lib.rs#L96-L105), a first-time registration with 0.005 != 0.00486 now panics with "Attached deposit must be exactly the storage cost …" instead of reaching the intended report_data-mismatch error. Test 6's assertion (test-script.js#L949-L959) requires the error to match /wrong report_data hash …/i, so it will now throw Expected WrongHash error with report_data, got: …. Net effect: the test fails and the negative attestation-verification path it was guarding is no longer exercised. The deposit should be lowered to the exact 0.00486 NEAR.

shade-agent-framework/tests-in-tee/test-image/src/tests/test-different-account-id.ts

Lines 85 to 99 in 8adec7f

	try {
	await differentAccount.callFunction({
	contractId: agentContractId,
	methodName: "register_agent",
	args: {
	attestation: contractAttestation,
	},
	deposit: NEAR.toUnits(0.005),
	gas: BigInt("300000000000000"), // 300 TGas
	});
	registrationError =
	"Registration with different account should have failed but succeeded";
	} catch (error: any) {
	registrationError = error.message || String(error);
	}

shade-agent-framework/tests-in-tee/test-script.js

Lines 948 to 959 in 8adec7f

	async (result) => {
	// Verify registrationError matches WrongHash format with report_data
	const registrationError = result.registrationError || "";
	if (
	!registrationError.match(
	/wrong report_data hash \(found .+ expected .+\)/i,
	)
	) {
	throw new Error(
	`Expected WrongHash error with report_data, got: ${registrationError}`,
	);
	}

2. [MEDIUM:50] forceDeposit: true on a re-registration now reliably fails against the default contract. register() attaches the default 0.00486 NEAR regardless of registration state when forceDeposit === true (skipping get_agent), but the contract now requires exactly 0 for an already-registered agent (lib.rs#L91-L102). Pre-PR >= accepted this. The common auto path correctly attaches 0 on re-registration, so this is an escape-hatch edge case — but the docs/reference/api.md forceDeposit: true row still documents it as valid "regardless of prior state" with no warning that it now fails on re-registration, and no unit test in api.test.ts covers it.

shade-agent-framework/shade-agent-js/src/api.ts

Lines 188 to 205 in 8adec7f

	let depositYocto: bigint;
	if (params?.forceDeposit === false) {
	depositYocto = 0n;
	} else if (params?.forceDeposit === true) {
	depositYocto = BigInt(
	params.deposit ?? DEFAULT_REGISTER_DEPOSIT_YOCTO,
	);
	} else {
	const existing = (await this.view({
	methodName: "get_agent",
	args: { account_id: this.agentAccountId },
	})) as SerializedReturnValue | null;
	const alreadyRegistered =
	existing !== null && existing !== undefined;
	depositYocto = alreadyRegistered
	? 0n
	: BigInt(params?.deposit ?? DEFAULT_REGISTER_DEPOSIT_YOCTO);
	}

3. [LOW:60] The contract's STORAGE_BYTES_TO_REGISTER and the JS DEFAULT_REGISTER_DEPOSIT_YOCTO are now coupled by an exact-match check but stay in sync only by manual convention. A future change to the contract's per-agent byte count (or NEAR's per-byte storage price) would silently break every default register() from a published shade-agent-js. The Rust side is guarded by test_agent_storage_cost_matches_expected, but nothing guards the hardcoded JS default against contract drift. The PR body acknowledges this brittleness; worth a comment cross-referencing the two constants at minimum.

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

Line 59 in 8adec7f

const STORAGE_BYTES_TO_REGISTER: u128 = 486;

shade-agent-framework/shade-agent-js/src/api.ts

Lines 50 to 51 in 8adec7f

	/** Default attached deposit for first-time `register_agent` when `deposit` is omitted: the exact storage cost the contract requires (0.00486 NEAR, yocto string). The contract requires this exact amount and never refunds, so attaching more would be rejected. */
	const DEFAULT_REGISTER_DEPOSIT_YOCTO = "4860000000000000000000";

[PiVortex]

/claude-review

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

[PiVortex]

Fixed in 4535748 — reworded to "A first-time register_agent must attach exactly this storage cost; re-registration attaches 0." so the comment no longer implies the deposit is always required.

[claude]

Code review

Overall this is a clean, well-synced change: register_agent now requires an exact deposit (== instead of >=) — exactly the storage cost for a first registration, exactly 0 for a re-registration, never refunding. The arithmetic checks out (486 × 1e19 yocto/byte = 0.00486 NEAR), the docs (agent-contract.md, api.md), the contract README, the Rust unit + integration tests, the JS unit test, and the one raw register_agent tests-in-tee scenario were all updated consistently. Attestation / measurement / PPID / compose-hash gating, owner-gating, and the signing path are untouched; the deposit check moving ahead of verify_attestation is benign (the whole tx still reverts before any state write). No CRITICAL or HIGH issues found.

Found 4 issues:

1. [LOW:70] Exact-match design couples the JS client to a value the contract derives dynamically. The contract requires attached_deposit() == agent_storage_cost() where agent_storage_cost() = env::storage_byte_cost() * STORAGE_BYTES_TO_REGISTER (a protocol-dependent value), but the client hardcodes the matching default. The switch from >= to == removes the upward tolerance the old check had, so if NEAR ever changes its per-byte storage cost, every default first-time register() reverts until clients are manually updated. The coupling is documented in comments and the contract side is guarded by test_agent_storage_cost_matches_expected, but the JS literal has only a doc-comment, no test guard — drift would surface as failed registrations in production rather than a failing build.

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

Lines 98 to 104 in 6b4ae88

	require!(
	env::attached_deposit() == required_deposit,
	&format!(
	"Attached deposit must be exactly {}",
	required_deposit.exact_amount_display()
	)
	);

shade-agent-framework/shade-agent-js/src/api.ts

Line 51 in 6b4ae88

const DEFAULT_REGISTER_DEPOSIT_YOCTO = "4860000000000000000000";

2. [MEDIUM:45] Published behavioral change to @neardefi/shade-agent-js without a version bump. The default deposit changed (0.005 → 0.00486 NEAR) and, because the new contract rejects overpayment, an un-upgraded client (old 0.005 default) will now fail to register against the new contract — yet package.json stays at 2.2.0. A version bump would signal to consumers that the client and contract must move together. (Not in CLAUDE.md's explicit update list and the per-PR bump convention is unclear, hence lower confidence — flagging for the maintainers' release process.)

shade-agent-framework/shade-agent-js/package.json

Line 3 in 6b4ae88

"version": "2.2.0",

3. [LOW:30] forceDeposit: true is now a foot-gun on re-registration. Re-registration must attach exactly 0, but forceDeposit: true always attaches a non-zero deposit, so calling register({ forceDeposit: true }) on an already-registered agent reverts against the default contract. This is correctly documented in api.md, so it's informational — but it's a subtle reversal for operators who use forceDeposit defensively on a refresh path.

shade-agent-framework/docs/reference/api.md

Lines 122 to 124 in 6b4ae88

	### How the attached deposit is chosen
	| `forceDeposit` | Behavior |

4. [LOW:25] test-different-account-id.ts hardcodes "0.00486" as a second copy of the storage cost rather than deriving it. Acceptable here because the test deliberately bypasses shade-agent-js with a raw NAJ call and only asserts that registration fails (so the deposit value isn't load-bearing), but it's another literal that can silently drift from the contract.

shade-agent-framework/tests-in-tee/test-image/src/tests/test-different-account-id.ts

Line 92 in 6b4ae88

deposit: NEAR.toUnits("0.00486"), // exact storage cost; the contract requires an exact deposit

[PiVortex]

Thanks — addressing the 4 findings from the latest review (head now 4535748):

1. [LOW:70] JS literal has no build-time guard — kept as-is. A shade-agent-js unit test would only re-assert the literal (it can't import the contract's STORAGE_BYTES_TO_REGISTER), so it wouldn't catch contract drift — just create a second copy to keep in sync. The real cross-check is the tests-in-tee successful-registration scenario: it calls register() with the default deposit against the freshly-deployed contract, so any drift between the JS default and the contract's exact cost fails there. Coupling is documented on both constants.
2. [MEDIUM:45] Version bump — by repo convention, published-package versions are bumped in dedicated chore(release): commits on main, not in feature PRs (now codified in CLAUDE.md via docs: add versioning and release-impact convention to CLAUDE.md #99). Release impact: @neardefi/shade-agent-js — minor (default register deposit 0.005 → 0.00486; backward-compatible for library upgraders since 0.00486 satisfies both the old >= and new == contracts, and it signals the coupled contract change). Stated in the PR description; the bump itself happens at release time.
3. [LOW:30] forceDeposit: true foot-gun on re-registration — already documented in api.md (the forceDeposit: true row + example note), which you flagged as informational. No code change.
4. [LOW:25] test-different-account-id.ts hardcodes 0.00486 — intentional: that test bypasses shade-agent-js with a raw NAJ call and must attach the exact amount to reach the report_data mismatch it asserts (a wrong value would trip the deposit check first). Acceptable, as you noted.

[PiVortex]

/claude-review

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

[PiVortex]

Keeping #[allow(dead_code)] — it's required, not removable. The helpers module is compiled into every integration-test binary, and owner_operations_tests / update_contract_tests include it but don't reference EXACT_STORAGE_DEPOSIT. Verified: removing the attribute produces constant EXACT_STORAGE_DEPOSIT is never used warnings in both of those test crates.

[claude]

Code review

This PR tightens register_agent's storage-deposit check from >= storage_cost (first-time only, re-registration unchecked) to an exact == required_deposit — agent_storage_cost() (0.00486 NEAR) for a first-time registration, 0 for a re-registration — and lowers the shade-agent-js default deposit from 0.005 to 0.00486 NEAR. Docs (agent-contract.md, api.md, README), unit/integration/tests-in-tee tests are all updated, a pub(crate) agent_storage_cost() helper plus a drift-guard test were added, and no package versions were bumped.

Security, access-control, attestation-verification ordering (the deposit require! panics and reverts before verify_attestation, so verification still runs unconditionally), secret-leakage, bug-scan (constants / zero-counts / should_panic strings / re-register test path all verified), and consumer-sync (shade-agent-template uses bare register(), CLI has no deposit references, removal logic confirms the auto-removed-agent docs note) all came back clean.

Found 3 issues:

1. [MEDIUM:75] Version-skew / coordinated-release hazard: moving to == makes an older shade-agent-js (default 0.005 NEAR) — or any caller passing an explicit deposit >= cost under the old "overpay is safe" semantics — fail first-time registration against the new contract, where it previously succeeded (5e21 != 4.86e21). The contract and client must now be released and pinned together. The docs say "attaching more is rejected," but there is no explicit migration note that an old client breaks against a freshly deployed template contract.

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

Lines 99 to 105 in 4535748

	require!(
	env::attached_deposit() == required_deposit,
	&format!(
	"Attached deposit must be exactly {}",
	required_deposit.exact_amount_display()
	)
	);

2. [MEDIUM:55] Exact-match couples a hardcoded client constant to a dynamic protocol parameter with zero headroom: the contract computes the required deposit at runtime as env::storage_byte_cost() * 486, but the client hardcodes DEFAULT_REGISTER_DEPOSIT_YOCTO = 0.00486 NEAR, which only equals it while storage_byte_cost stays at 1e19 yocto/byte. If a NEAR protocol upgrade ever changes storage_byte_cost, the contract's required exact amount shifts but the hardcoded client default does not — breaking all first-time registrations, whereas the old >= check absorbed an increase. Real but rare (the parameter has been stable for years); the coupling is acknowledged in the lib.rs / api.ts comments.

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

Lines 142 to 146 in 4535748

	pub(crate) fn agent_storage_cost() -> NearToken {
	env::storage_byte_cost()
	.checked_mul(STORAGE_BYTES_TO_REGISTER)
	.unwrap()
	}

3. [LOW:20] forceDeposit: true now always fails on an already-registered agent (it always attaches a non-zero deposit, but re-registration requires exactly 0). This is an inherent consequence of the exact-match design and is now documented in api.md ("Only safe for a first-time registration") — informational, not a defect.

shade-agent-framework/shade-agent-js/src/api.ts

Lines 191 to 194 in 4535748

	} else if (params?.forceDeposit === true) {
	depositYocto = BigInt(
	params.deposit ?? DEFAULT_REGISTER_DEPOSIT_YOCTO,
	);

[PiVortex]

Thanks — addressing the 3 findings (head now c4491d7):

1. [MED:75] Missing migration note — added in c4491d7: an upgrade-coordination callout in the register_agent reference (docs/reference/agent-contract.md) noting that a client attaching the old default (0.005 NEAR) fails first-time registration against the exact-deposit contract, so the contract and shade-agent-js must be deployed together.
2. [MED:55] Exact-match couples the client constant to storage_byte_cost — this is the intended design tradeoff (exact deposit, chosen over >=+refund), and the zero-headroom brittleness is documented in the lib.rs / api.ts comments and the PR's brittleness note. No code change; the tests-in-tee successful-registration scenario is the cross-check if the two constants ever drift.
3. [LOW:20] forceDeposit: true on re-registration — informational, already documented in api.md (the forceDeposit: true row is marked first-registration-only). No change.

[PiVortex]

/claude-review

[Copilot]

Pull request overview

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

[claude]

Code review

Found 3 issues (all LOW — no CRITICAL/HIGH/MEDIUM survived scrutiny). The core change is solid: the attestation verification path in register_agent is untouched and still runs in full (a wrong deposit just panics and rolls back the whole tx before verify_attestation), access control is unchanged, all five copies of the constant (486 bytes × 1e19 = 4_860_000_000_000_000_000_000 yocto = 0.00486 NEAR — in lib.rs, api.ts, both Rust test helpers, and the tests-in-tee file) agree, and the docs / unit + integration tests / shade-agent-template are in sync per the repo's cross-package rules. No version was bumped. The two new negative tests and their #[should_panic] messages are correct.

1. [LOW:55] The new remove_agent NOTE's parenthetical "(e.g. an expired attestation)" can mislead. An expired attestation does not eagerly evict the agent from the agents map — removal is lazy and only happens inside require_valid_agent (helpers.rs:33-34), which is triggered by a signature request, not by registration. So an agent whose attestation merely expired and then calls register_agent to refresh is still present in the map → already_registered == true → required deposit is 0, not the storage cost. The "pay the storage cost again" path only applies after the agent was actually removed (manual remove_agent, or auto-removal triggered by a signing attempt). The contract and JS client agree (both key on map presence), so there is no functional bug — just doc imprecision worth tightening.

shade-agent-framework/docs/reference/agent-contract.md

Line 125 in c4491d7

> A removed agent can re-register by calling `register_agent` with a valid attestation. Removal does not refund the storage deposit, so a re-registration is treated as a first-time registration and must attach the storage cost again. The same applies when an agent is auto-removed for becoming invalid (e.g. an expired attestation).

2. [LOW:50] Moving register_agent from >= storage_cost to == required_deposit is a breaking behavioral change that requires the contract and every client to be upgraded in lockstep, and it hard-couples the JS literal DEFAULT_REGISTER_DEPOSIT_YOCTO to the Rust STORAGE_BYTES_TO_REGISTER constant with no cross-package runtime guard — if the per-agent storage cost ever changes, an un-updated client silently fails at registration. This is acknowledged and well-mitigated (coupled-release note, the [!IMPORTANT] callout in agent-contract.md, bidirectional source comments, and the test_agent_storage_cost_matches_expected drift guard on the contract side). Flagged as a documented design trade-off, not a defect. Note shade-contract-template isn't in the repo's published-versioned-package list, so a contract-side release-impact line isn't strictly required, but the change is breaking for any deployed contract + client pair.

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

Line 62 in c4491d7

const STORAGE_BYTES_TO_REGISTER: u128 = 486;

3. [LOW:40] test_agent_storage_cost_matches_expected is largely tautological — EXACT_STORAGE_DEPOSIT and agent_storage_cost() are both 486 × storage_byte_cost, so the assertion mainly guards the hardcoded test constant against drift rather than independently validating the 486-byte figure. The real end-to-end proof that 0.00486 NEAR exactly covers the storage is the sandbox integration test test_register_agent_new_agent_requires_storage_deposit_integration. Minor; coverage is adequate overall.

shade-agent-framework/shade-contract-template/src/internal/unit_tests.rs

Line 475 in c4491d7

fn test_agent_storage_cost_matches_expected() {

[PiVortex]

Thanks — addressing the 3 LOW findings (head now e5d6c9c):

1. [LOW:55] remove_agent note imprecision — fixed in e5d6c9c. Reworded the note: removal is manual (remove_agent) or lazy (an invalid agent is evicted only when it next calls an agent-gated method, not by register_agent), and an expired-but-still-mapped agent re-registers with a 0 deposit. Good catch on the eager-vs-lazy distinction.
2. [LOW:50] Breaking/coupling tradeoff — agreed; documented design tradeoff, not a defect (coupled-release note, the [!IMPORTANT] callout, bidirectional source comments, and the test_agent_storage_cost_matches_expected drift guard). No change.
3. [LOW:40] Tautological drift-guard test — that's its intended purpose (guard the JS-mirrored literal against drift); the real end-to-end cost proof is the sandbox test_register_agent_new_agent_requires_storage_deposit_integration, as you note. No change.

[PiVortex]

/claude-review

[Copilot]

Pull request overview

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

[claude]

Code review

Found 2 issues (both low/medium, well-mitigated — no blocking concerns):

1. [MEDIUM:45] Cross-package value drift is now a hard failure mode. The exact == deposit check ties first-time registration to the contract helper agent_storage_cost(), which recomputes live from the protocol value env::storage_byte_cost(). But the client default in shade-agent-js (DEFAULT_REGISTER_DEPOSIT_YOCTO = "4860000000000000000000") and the Rust test constants (EXACT_STORAGE_DEPOSIT) are hardcoded. Under the old >= check a JS-side under/over-estimate was absorbed; under ==, any drift — for example if NEAR ever changes the per-byte storage price, or STORAGE_BYTES_TO_REGISTER changes without the JS side being updated — hard-fails every first-time registration. This is well-mitigated (the test_agent_storage_cost_matches_expected unit test guards the constant against byte-cost drift, both sides carry cross-reference comments, and the docs add an explicit "upgrade contract + clients together" note), but there is no automated check coupling the Rust 486 × storage_byte_cost to the JS 4860000000000000000000 — only the live tests-in-tee registration would catch a cross-language desync.

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

Line 62 in e5d6c9c

const STORAGE_BYTES_TO_REGISTER: u128 = 486;

shade-agent-framework/shade-agent-js/src/api.ts

Line 51 in e5d6c9c

const DEFAULT_REGISTER_DEPOSIT_YOCTO = "4860000000000000000000";

2. [LOW:40] == removes the ability of a client to cover a storage underestimate. Previously a client could overpay (the >= headroom) to cover any case where 486 bytes underestimates real per-agent storage; the contract just kept the excess. With the exact check, if STORAGE_BYTES_TO_REGISTER is ever too low, the contract silently subsidizes the shortfall from its own balance and clients have no way to compensate. The 486 value is unchanged by this PR and the Agent fields are fixed-size, so this is a latent property rather than an active bug — flagging only because the new semantics make any future under-sizing unrecoverable on the client side instead of client-correctable.

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

Lines 100 to 104 in e5d6c9c

	env::attached_deposit() == required_deposit,
	&format!(
	"Attached deposit must be exactly {}",
	required_deposit.exact_amount_display()
	)

---

Everything else checks out. Reviewed for security, architecture/cross-package sync, bugs, and production concerns:

• Attestation & access control unchanged — verify_attestation, owner-gating, whitelist gating, require_valid_agent lazy eviction, and the chain-signature path are untouched. The deposit require! still runs before verify_attestation exactly as before (no reordering). register_agent still binds to env::predecessor_account_id() (via the predecessor local). No new unauthenticated path, no pay-nothing / eviction-evasion path.
• No secrets introduced in code, tests, fixtures, or the new "Attached deposit must be exactly {}" message.
• Numeric constants verified — 486 × 1e19 = 4_860_000_000_000_000_000_000 yocto = 0.00486 NEAR, matching the Rust const and JS string; DEPOSIT_BELOW_COST (0.003) and DEPOSIT_ABOVE_COST (0.005) correctly straddle it. New tests (..._exceeds_cost, ..._reregister_attaches_deposit, ..._matches_expected) are sound, and all #[should_panic] strings match the new prefix.
• Cross-package sync rules satisfied — contract change maps to docs/reference/agent-contract.md; shade-agent-js change maps to docs/reference/api.md plus a unit test; tests-in-tee/test-different-account-id.ts (the only scenario attaching an explicit deposit) updated; all other consumers use agent.register() default. shade-agent-template (bare register()) and shade-agent-cli (no register-deposit references) correctly need no change.
• No version bumps in the diff (per repo convention); release impact is stated in the PR description. agent_storage_cost() is pub(crate) in a non-#[near] impl block, so it is correctly not exposed as a contract method. No checked_mul().unwrap() overflow risk (compile-time const × protocol value, not attacker-influenced).

[PiVortex]

Keep the previous statement, A removed agent can re-register by calling register_agent with a valid attestation.

[PiVortex]

remove this note

[PiVortex]

Superseded by #103. Closing this in favor of a require-≥-and-refund-the-excess design instead of the exact == deposit here: the contract requires at least the storage cost (0 on re-registration) and refunds any excess, so callers can safely over-attach. That removes the client↔contract exact-match coupling / version-skew this PR's reviews kept flagging, and leaves the sandbox + tests-in-tee deposits unchanged (0.005 ≥ 0.00486). #103 closes #64.