Gas fee optimization

[aniket866]

This PR addresses Following change:

In simple terms previously we are reading from :

Gas Waste (Before) The original function accessed storage fields 14 times during execution. Each access triggers an SLOAD operation (expensive).

• invoice.to: Accessed 2 times (Auth check, Event)
• invoice.isPaid: Accessed 1 time (Check)
• invoice.isCancelled: Accessed 1 time (Check)
• invoice.tokenAddress: Accessed 4 times (Logic checks, Transfer, Event)
• invoice.amountDue: Accessed 4 times (Value check, Allowance, Transfer, Event)
• invoice.from: Accessed 2 times (Transfer, Event)

Total Storage Reads: ~14 SLOADs (Warm access = 100 gas each) -> ~1,400 Gas overhead

#after:

• Storage Reads: Performed only 1 time (to copy struct to memory).
• Memory Reads: All 14 logic checks now use MLOAD (3 gas each).

Savings: ~1,350+ Gas per transaction.

@kumawatkaran523 Please checkout this Optimization, thankyou

Summary by CodeRabbit

• Chores
  • Reduced gas costs for invoice payment transactions by optimizing internal data access patterns.
  • No changes to user-visible behavior or public interfaces; payment flow and outcomes remain the same.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The payInvoice function in Chainvoice.sol now loads InvoiceDetails into a storage variable (invoiceStorage) first, then copies it to a memory variable (invoice) for checks; only isPaid is written back to storage. Authorization, cancellation, and payment logic are unchanged.

Changes

Cohort / File(s)	Summary
Storage Data Access Refactor contracts/src/Chainvoice.sol	payInvoice now reads the invoice into a storage struct (invoiceStorage) then copies to a memory struct (invoice) for validation; only invoiceStorage.isPaid is updated in storage.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly Related Issues

• Gas Fees Optimization: Reduce Storage Reads #74 — Same change pattern: caching invoice data in memory after a storage read and only updating isPaid in storage.

Poem

🐰 I hop through structs with nimble paws,
Read to storage, copy with cause,
Checks in memory, writes kept light,
Invoices paid with tidy flight. ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main objective of the PR, which is to optimize gas fees by reducing storage reads through a more efficient data-access pattern.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)

contracts/src/Chainvoice.sol (1)

206-213: Critical bug: isPaid state change is never persisted to storage.

The memory copy invoice is modified on line 213, but this change is never written back to storage. The storage variable invoiceStorage remains unchanged, meaning isPaid stays false permanently.

Impact: The same invoice can be paid an unlimited number of times since the require(!invoice.isPaid, "Already paid") check will always pass. This allows attackers to drain funds from payers.

🔒 Proposed fix

         InvoiceDetails storage invoiceStorage = invoices[invoiceId]; //(read once from storage)
         InvoiceDetails memory invoice = invoiceStorage;   // now read all from this invoice(memory)
         require(msg.sender == invoice.to, "Not authorized");
         require(!invoice.isPaid, "Already paid");
         require(!invoice.isCancelled, "Invoice is cancelled");

         // Effects first for CEI (mark paid, bump fees), then interactions
-        invoice.isPaid = true;
+        invoiceStorage.isPaid = true;

Note: Compare with the correct pattern used in payInvoicesBatch at line 281: invoices[invoiceIds[i]].isPaid = true which writes directly to storage.