Skip to content

fix(factory): use checked_add for PoolCount, add PoolCountOverflow er… #82 - #104

Open
musameshack461-star wants to merge 1 commit into
SmartDropLabs:mainfrom
musameshack461-star:fix/82-pool-count-checked-add
Open

fix(factory): use checked_add for PoolCount, add PoolCountOverflow er… #82#104
musameshack461-star wants to merge 1 commit into
SmartDropLabs:mainfrom
musameshack461-star:fix/82-pool-count-checked-add

Conversation

@musameshack461-star

Copy link
Copy Markdown

closes #82

  • Replace bare pool_id + 1 with checked_add(1).ok_or(PoolCountOverflow)? so overflow is caught as a typed contract error regardless of the Cargo profile's overflow-checks setting.
  • Add FactoryError::PoolCountOverflow = 8 to types.rs.
  • checked_add is evaluated before deploy_v2, so no deployment side effect can occur at the overflow boundary.
  • Add test_create_pool_at_max_pool_count_returns_overflow_error: seeds PoolCount to u32::MAX via env.as_contract, asserts the typed error is returned, and verifies PoolCount is not mutated on failure.
  • Remove stale factory: create_pool deploys the pool via deploy_v2(wasm_hash, ()) without ever calling initialize, leaving every deployed pool permanently unusable #78 workarounds from factory_pool_integration.rs: two tests were manually calling pool_client.initialize after create_pool, which is now wrong since create_pool initializes pools in the same transaction. Also fix the daily_rate in end_to_end_create_pool_then_lock_and_unlock from 300 (below LEDGERS_PER_DAY) to 34_560 (2 * 17_280 = credit_rate 2).

All 45 unit tests and 3 integration tests pass.

…ror (SmartDropLabs#82)

- Replace bare pool_id + 1 with checked_add(1).ok_or(PoolCountOverflow)?
  so overflow is caught as a typed contract error regardless of the Cargo
  profile's overflow-checks setting.
- Add FactoryError::PoolCountOverflow = 8 to types.rs.
- checked_add is evaluated before deploy_v2, so no deployment side effect
  can occur at the overflow boundary.
- Add test_create_pool_at_max_pool_count_returns_overflow_error: seeds
  PoolCount to u32::MAX via env.as_contract, asserts the typed error is
  returned, and verifies PoolCount is not mutated on failure.
- Remove stale SmartDropLabs#78 workarounds from factory_pool_integration.rs: two tests
  were manually calling pool_client.initialize after create_pool, which is
  now wrong since create_pool initializes pools in the same transaction.
  Also fix the daily_rate in end_to_end_create_pool_then_lock_and_unlock
  from 300 (below LEDGERS_PER_DAY) to 34_560 (2 * 17_280 = credit_rate 2).

All 45 unit tests and 3 integration tests pass.
@netlify

netlify Bot commented Jul 21, 2026

Copy link
Copy Markdown

Deploy Preview for sdcontracts ready!

Name Link
🔨 Latest commit 7a0afb8
🔍 Latest deploy log https://app.netlify.com/projects/sdcontracts/deploys/6a5f7de5c7ed01000841e83f
😎 Deploy Preview https://deploy-preview-104--sdcontracts.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

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.

factory: PoolCount increment via pool_id + 1 has no checked_add, risking wraparound and clobbering pool 0's record

2 participants