Skip to content

chore: exempt network_writer on attestation#1255

Merged
MicBun merged 1 commit into
mainfrom
exemptAttes
Nov 7, 2025
Merged

chore: exempt network_writer on attestation#1255
MicBun merged 1 commit into
mainfrom
exemptAttes

Conversation

@MicBun
Copy link
Copy Markdown
Contributor

@MicBun MicBun commented Nov 7, 2025

resolves: https://github.com/trufnetwork/truf-network/issues/1336

Summary by CodeRabbit

  • New Features
    • Added role-based fee exemption for certain users in attestation requests
    • Updated conditional fee structure with 40 TRUF charge for applicable callers

@MicBun MicBun requested a review from outerlook November 7, 2025 11:16
@MicBun MicBun self-assigned this Nov 7, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 7, 2025

Walkthrough

This change removes an explicit runtime permission requirement for system:network_writer in the attestation request action, replacing it with a conditional fee-exemption pathway. Callers with system:network_writer membership are now exempt from the 40 TRUF attestation fee, enabling testing scenarios that previously required funded accounts.

Changes

Cohort / File(s) Summary
Attestation Fee Exemption Logic
internal/migrations/024-attestation-actions.sql
Introduced role-based fee exemption by replacing upfront permission check with exemption flag based on system:network_writer membership. Added fee variables and converted fee collection to conditional flow: exempt callers skip the 40 TRUF fee and balance validation; non-exempt callers incur fees with max_fee validation and leader address transfer. Deterministic execution and attestation storage flow remain unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Fee exemption logic correctness and potential edge cases around balance checks when exempt
  • Conditional transfer logic to ensure fees only move when applicable
  • Interaction between exemption flag and existing max_fee validation

Possibly related PRs

Suggested labels

type: chore

Suggested reviewers

  • outerlook
  • williamrusdyputra

Poem

🐰 A writer exempt from the fee,
No forty TRUF need be,
Testing flows free at last,
No balance blockers—fast!
The network's doors now open wide,
For all who test with honest pride.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: exempt network_writer on attestation' is specific and directly related to the main change: implementing exemption logic for the network_writer role in attestation fee collection.
Linked Issues check ✅ Passed The PR successfully implements the whitelist mechanism by exempting network_writer role holders from the 40 TRUF fee, addressing the need to test attestation without funding in CI/CD and leader wallet scenarios.
Out of Scope Changes check ✅ Passed All changes are focused on implementing fee exemption for network_writer role in the attestation migration; no unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch exemptAttes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@holdex
Copy link
Copy Markdown

holdex Bot commented Nov 7, 2025

Time Submission Status

Member Status Time Action Last Update
@MicBun ❌ Missing - ⚠️ Submit time -
@outerlook ❌ Missing - ⚠️ Submit time -

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal/migrations/024-attestation-actions.sql (1)

5-13: Update permissions documentation to reflect new fee-exemption model.

The docstring is now inaccurate. The code no longer enforces an exclusive permission check; instead, it allows any wallet to request attestations, with system:network_writer members exempt from the 40 TRUF fee. Non-members must pay the fee. Update the Permissions section to reflect this.

Suggested update:

 /**
  * request_attestation: Request signed attestation of query results
  *
  * Permissions:
- * - Only wallets with the 'system:network_writer' role can request attestations.
+ * - Any wallet can request attestations.
+ * - Wallets with 'system:network_writer' role are exempt from the 40 TRUF fee.
+ * - Non-exempt wallets must pay a 40 TRUF flat fee for attestation processing.
  *
  * Validates action is allowed, executes query deterministically, calculates
  * attestation hash, stores unsigned attestation, and queues for signing.
  */
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 313a38a and bb96afe.

📒 Files selected for processing (1)
  • internal/migrations/024-attestation-actions.sql (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-10T13:00:14.189Z
Learnt from: outerlook
Repo: trufnetwork/node PR: 1207
File: internal/migrations/024-attestation-actions.sql:58-76
Timestamp: 2025-10-10T13:00:14.189Z
Learning: In the attestation system for internal/migrations/024-attestation-actions.sql, the attestation_hash is computed from (version|algo|data_provider|stream_id|action_id|args) and intentionally excludes created_height. This design ensures the hash is deterministic based only on user input, not network state like block height.

Applied to files:

  • internal/migrations/024-attestation-actions.sql
📚 Learning: 2025-10-10T13:00:13.731Z
Learnt from: outerlook
Repo: trufnetwork/node PR: 1207
File: internal/migrations/023-attestation-schema.sql:20-21
Timestamp: 2025-10-10T13:00:13.731Z
Learning: In the attestations table (internal/migrations/023-attestation-schema.sql), the primary key is (requester, created_height, attestation_hash) because attestation_hash is computed deterministically from user input only (version|algo|data_provider|stream_id|action_id|args) and does not include created_height. This allows the same user to request the same attestation at different block heights.

Applied to files:

  • internal/migrations/024-attestation-actions.sql
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: acceptance-test
🔇 Additional comments (1)
internal/migrations/024-attestation-actions.sql (1)

48-53: No issues found—code is correct.

The are_members_of() function is properly defined in migration 013 with the expected contract: it returns TABLE (wallet TEXT, is_member BOOL), accepts the 'system' owner, and the 'system:network_writer' role is bootstrapped in migration 015 before migration 024 executes. Non-existent wallets correctly return is_member=FALSE rather than being excluded. The function's helper validations pass with the hardcoded role name and owner provided at line 48.

Comment thread internal/migrations/024-attestation-actions.sql
@MicBun MicBun merged commit 1117c3d into main Nov 7, 2025
4 of 7 checks passed
@MicBun MicBun deleted the exemptAttes branch November 7, 2025 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants