Skip to content

chore: add transaction fees for attestation requests#1247

Merged
williamrusdyputra merged 2 commits into
mainfrom
attestationFee
Nov 4, 2025
Merged

chore: add transaction fees for attestation requests#1247
williamrusdyputra merged 2 commits into
mainfrom
attestationFee

Conversation

@MicBun
Copy link
Copy Markdown
Contributor

@MicBun MicBun commented Nov 3, 2025

Data attestation queries are currently free, creating no cost barrier for cryptographic verification services. This introduces a 40 TRUF flat fee per attestation request to monetize this premium service while ensuring only serious usage of the attestation system.

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

Summary by CodeRabbit

  • New Features

    • Attestation requests now require a 40 TRUF fee; caller balance is validated and fees are transferred to designated leaders.
  • Tests

    • Added end-to-end tests covering fee charging, insufficient-balance errors, repeated requests, leader payouts, and balance correctness.
    • Test helpers updated to support ERC20 balance setup, leader proposer contexts, and utilities to credit test balances.

Data attestation queries are currently free, creating no cost barrier for cryptographic verification services. This introduces a 40 TRUF flat fee per attestation request to monetize this premium service while ensuring only serious usage of the attestation system.

resolves: trufnetwork/truf-network#1316
@MicBun MicBun self-assigned this Nov 3, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 3, 2025

Walkthrough

Adds mandatory on-chain fee collection (fixed 40 TRUF) into the request_attestation SQL action: checks caller ERC20 balance, enforces insufficient-funds error, and transfers the fee to the leader before continuing with attestation logic. Includes tests and test-helper updates to exercise fee behavior.

Changes

Cohort / File(s) Summary
Fee collection SQL
internal/migrations/024-attestation-actions.sql
Inserts a fee collection block into request_attestation: reads caller ERC20 balance, enforces >=40 TRUF, validates @leader_sender not NULL, and transfers 40 TRUF to leader before attestation proceeds.
New end-to-end tests
tests/streams/attestation/request_attestation_fee_test.go
Adds kwiltest E2E tests covering fee payment, insufficient-balance errors, repeated requests, leader receives fees, and balance assertions.
Test helpers & existing tests updated
tests/streams/attestation/test_helpers.go, tests/streams/attestation/attestation_request_test.go
Adds ERC20 instance support, leader public key field, GiveBalance helper, and updates contexts/authenticator (Proposer, EthPersonalSignAuth) so tests can simulate balances and leader fee receipt. Minor test context adjustments in attestation_request_test.go.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant RequestAtt as request_attestation
    participant EthBridge as ethereum_bridge
    participant Leader
    participant AttLogic as Attestation Logic

    Caller->>RequestAtt: invoke request_attestation(...)
    RequestAtt->>RequestAtt: validate action_id / role checks

    rect rgb(235,245,255)
      Note over RequestAtt,EthBridge: FEE COLLECTION (new)
      RequestAtt->>EthBridge: balance(`@caller`)
      EthBridge-->>RequestAtt: balance amount
      alt balance < 40 TRUF
        RequestAtt-->>Caller: error "insufficient balance"
      else balance >= 40 TRUF
        RequestAtt->>RequestAtt: ensure `@leader_sender` NOT NULL
        RequestAtt->>EthBridge: transfer(40 TRUF -> `@leader_sender`)
        EthBridge->>Leader: +40 TRUF
      end
    end

    RequestAtt->>AttLogic: continue attestation flow
    AttLogic-->>Caller: attestation result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing extra attention:
    • internal/migrations/024-attestation-actions.sql — SQL control-flow placement, error strings, and correctness of ERC20 bridge calls.
    • tests/streams/attestation/request_attestation_fee_test.go — test setup/teardown, correct use of ERC20 instance IDs, and deterministic balances.
    • tests/streams/attestation/test_helpers.go — new leader key handling and GiveBalance integration with test chain helpers.

Possibly related PRs

Suggested reviewers

  • outerlook
  • williamrusdyputra

Poem

🐇
Forty TRUF hops from wallet to lead,
A little fee for verified deed,
Balances checked, the transfer made,
Leader pockets what callers paid,
Now attestations hop on, unafraid. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and concisely describes the main change: adding transaction fees for attestation requests, which is the primary objective of this changeset.
Linked Issues check ✅ Passed The PR implements all coding requirements from issue #1316: fee collection added post-role check, balance validation with 40 TRUF minimum, fee transfer to @leader_sender, flat fee structure, and existing logic preservation.
Out of Scope Changes check ✅ Passed All changes are scoped to fee collection implementation: SQL migration adds fee logic, Go tests validate fee behavior, and test helpers support balance operations. No unrelated functionality or refactoring detected.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch attestationFee

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ecbdbca and 47a3dbc.

📒 Files selected for processing (2)
  • tests/streams/attestation/attestation_request_test.go (2 hunks)
  • tests/streams/attestation/test_helpers.go (8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-12T12:03:13.782Z
Learnt from: outerlook
Repo: trufnetwork/node PR: 1147
File: tests/streams/utils/erc20/helper.go:37-41
Timestamp: 2025-09-12T12:03:13.782Z
Learning: In the ERC20 bridge test helpers at tests/streams/utils/erc20/helper.go, the extensionAlias parameter serves dual purpose as both the extension identifier and the chain name. This is a deliberate design choice in their test setup where extensionAlias and chain are the same value.

Applied to files:

  • tests/streams/attestation/test_helpers.go
🧬 Code graph analysis (1)
tests/streams/attestation/test_helpers.go (1)
extensions/tn_utils/serialization.go (1)
  • EncodeActionArgs (45-76)
⏰ 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). (2)
  • GitHub Check: lint
  • GitHub Check: acceptance-test
🔇 Additional comments (3)
tests/streams/attestation/attestation_request_test.go (1)

148-155: Fee-aware unauthorized context scaffolding looks solid. The added proposer and EthPersonalSignAuth wiring keep the role-gate test relevant under the new fee path while avoiding balance setup surprises, nice work.

tests/streams/attestation/test_helpers.go (2)

98-113: App helper wiring cleanly provisions balances. Prefunding the deployer via the ERC20 test hooks ensures every attestation call can cover the 40 TRUF fee without cluttering individual tests—great reuse of the bridge utilities.


329-335: New balance helper keeps scenarios terse. Centralizing the credit logic (with the ForTesting credit path) makes the fee tests far easier to read and evolve; thanks for the tidy abstraction.


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 3, 2025

Time Submission Status

Member Status Time Action Last Update
MicBun ✅ Submitted 6h Update time Nov 4, 2025, 9:04 AM
williamrusdyputra ✅ Submitted 10min Update time Nov 4, 2025, 9:05 AM

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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ef71f05 and ecbdbca.

📒 Files selected for processing (2)
  • internal/migrations/024-attestation-actions.sql (1 hunks)
  • tests/streams/attestation/request_attestation_fee_test.go (1 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
🧬 Code graph analysis (1)
tests/streams/attestation/request_attestation_fee_test.go (8)
tests/streams/utils/runner.go (1)
  • RunSchemaTest (37-116)
internal/migrations/migration.go (1)
  • GetSeedScriptStatements (60-109)
tests/streams/utils/utils.go (1)
  • GetTestOptionsWithCache (75-88)
tests/streams/utils/setup/roles.go (1)
  • AddMemberToRoleBypass (73-98)
tests/streams/utils/setup/common.go (4)
  • CreateDataProvider (193-234)
  • CreateStream (31-33)
  • StreamInfo (21-24)
  • ContractTypePrimitive (17-17)
tests/streams/utils/erc20/inject.go (1)
  • InjectERC20Transfer (24-85)
tests/streams/utils/erc20/helper.go (1)
  • GetUserBalance (49-74)
extensions/tn_utils/serialization.go (1)
  • EncodeActionArgs (45-76)
⏰ 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). (2)
  • GitHub Check: acceptance-test
  • GitHub Check: lint

Comment thread internal/migrations/024-attestation-actions.sql
Copy link
Copy Markdown
Contributor

@williamrusdyputra williamrusdyputra left a comment

Choose a reason for hiding this comment

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

coderabbit suggestion, is it unrelevant?

and also the CI failed

@MicBun
Copy link
Copy Markdown
Contributor Author

MicBun commented Nov 4, 2025

Thanks for notifying. It is valid and will push fix soon

@MicBun MicBun marked this pull request as draft November 4, 2025 06:59
@MicBun MicBun marked this pull request as ready for review November 4, 2025 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants