fix: minor issues and docs

[fnanni-0]

Summary by CodeRabbit

• Bug Fixes

  • Corrected array indexing bounds in bid retrieval function to properly enumerate bids.
  • Removed payable acceptance from payment execution function; payments now derived solely from contract state.

• Documentation

  • Enhanced ranking system documentation to clarify point invariants, tie-breaking, and reward distribution.
  • Expanded auction mechanics documentation including linked-list structure and settlement behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

Documentation and logic updates across three contracts. Market.sol refines ranking semantics through comments. Billing.sol removes the payable modifier from executePayment, preventing Ether deposits. FirstPriceAuction.sol adds NatSpec documentation and fixes the getBids iteration logic to correctly index results.

Changes

Cohort / File(s)	Summary
Market Ranking Documentation contracts/Market.sol	Updated registerPoints semantics to clarify no-op behavior for lower-scoring tokens and placement within equal-score groups. Refactored registerAll documentation to reflect skipped timeout and immediate claiming availability. Expanded claimRewards and getReward documentation to detail exact boundary matching and prize weight redistribution. No public signature changes.
Billing Payment Function contracts/ads/Billing.sol	Removed payable modifier from executePayment(IMarket _market), preventing callers from sending Ether; payout now driven solely by stored balance state.
FirstPriceAuction Documentation & Loop Fix contracts/ads/FirstPriceAuction.sol	Added comprehensive NatSpec for contract, constants, and functions describing linked-list mechanics and settlement behavior. Enhanced placeBid/removeBid/executeHighestBid documentation with control-flow details. Fixed getBids loop: changed bounds from i <= _to to i < _to, added early break on nextID == 0x0, corrected indexing to bidsArray[i - _from], and removed unused currentID variable.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

📚 Our docs grew clear, the ranking refined,
No ether flows where none was signed,
The bid-list loops now count just right,
Each line of change brings clearer sight! 🐰✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'fix: minor issues and docs' is vague and uses non-descriptive terms that don't convey meaningful information about the specific changes across three contracts.	Revise the title to be more specific and descriptive of the main changes, such as 'refactor: update ranking logic docs and remove payable from executePayment' or 'docs: clarify Market ranking behavior and FirstPriceAuction settlement logic'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
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.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/minor-issues-and-docs

---

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: 1

Caution

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

⚠️ Outside diff range comments (1)

contracts/Market.sol (1)

336-346: ⚠️ Potential issue | 🟠 Major

Clear old ranking slots before writing the full recomputation.

If anyone already called registerPoints() earlier in the submission window, this loop only overwrites [0, freePos) and leaves stale ranking[...] entries behind. claimRewards() reads ranking[_lastSharedIndex + 1], and getReward() scans tail entries for vacant prizes, so leftover non-zero points can block valid claims or skew payout splitting.

🛠️ Suggested fix

-        // Write the final ranking to storage.
+        // Clear any partially submitted ranking state before materializing the full ranking.
+        for (uint256 rankIndex = 0; rankIndex < rankingLength; rankIndex++) {
+            delete ranking[rankIndex];
+        }
+
+        // Write the final ranking to storage.
         for (uint256 rankIndex = 0; rankIndex < freePos; rankIndex++) {
             uint256 tokenID = auxRanking[rankIndex] >> 128;
             uint256 totalPoints = auxRanking[rankIndex] & CLEAN_TOKEN_ID;
             ranking[rankIndex].tokenID = tokenID;
             ranking[rankIndex].points = uint248(totalPoints);
             emit RankingUpdated(tokenID, totalPoints, rankIndex);
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@contracts/Market.sol` around lines 336 - 346, The loop writes new entries
only for indexes [0, freePos) but leaves stale entries beyond freePos which
breaks claimRewards() and getReward(); fix by reading the previous ranking
length into a local (e.g., uint256 oldLen = ranking.length) before overwriting,
then after populating [0, freePos) explicitly clear any remaining slots for
indexes [freePos, oldLen) by setting ranking[i].tokenID = 0 and
ranking[i].points = 0 (and emit RankingUpdated if you want consistent events),
so registerPoints(), claimRewards(), and getReward() see vacant slots correctly;
update any invariants around resultSubmissionPeriodStart unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@contracts/ads/FirstPriceAuction.sol`:
- Around line 321-331: The getBids function currently allocates Bid[] memory
bidsArray = new Bid[](_to - _from) before validating inputs, which will panic if
_from > _to; add explicit input checks at the start of getBids: require(_from <=
_to, "invalid pagination range") and require(_to <= totalBids, "pagination out
of bounds") where totalBids is the length of the bids list for the given _market
(e.g., use the storage array or mapping that holds bids for _market), then
proceed to allocate bidsArray and copy entries; reference getBids and the
bidsArray allocation so the validation is performed before new Bid[](...) and
any indexing.

---

Outside diff comments:
In `@contracts/Market.sol`:
- Around line 336-346: The loop writes new entries only for indexes [0, freePos)
but leaves stale entries beyond freePos which breaks claimRewards() and
getReward(); fix by reading the previous ranking length into a local (e.g.,
uint256 oldLen = ranking.length) before overwriting, then after populating [0,
freePos) explicitly clear any remaining slots for indexes [freePos, oldLen) by
setting ranking[i].tokenID = 0 and ranking[i].points = 0 (and emit
RankingUpdated if you want consistent events), so registerPoints(),
claimRewards(), and getReward() see vacant slots correctly; update any
invariants around resultSubmissionPeriodStart unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e1d072cd-d19b-453b-9453-c8d56715b250

📥 Commits

Reviewing files that changed from the base of the PR and between 75a7ba5 and 55e469d.

📒 Files selected for processing (3)

• contracts/Market.sol
• contracts/ads/Billing.sol
• contracts/ads/FirstPriceAuction.sol