chores:added a revamp to the contract and integration test

[Samuel1-ona]

BigInc Genesis Contract

This PR closes #13

Overview

The BigInc Genesis contract is a comprehensive share management system built on Starknet that allows users to purchase shares using USDT or USDC tokens. The contract includes features for presale management, partner share caps, shareholder tracking, and administrative controls.

🚀 Recent Changes & Revamp

Major Contract Refactoring

The contract has been completely revamped with the following improvements:

1. Simplified Architecture

• Removed OpenZeppelin Components: Eliminated dependency on OwnableComponent, PausableComponent, and ReentrancyGuardComponent
• Custom Implementation: Implemented ownership and pausing functionality directly in the contract
• Cleaner Storage: Streamlined storage structure with better organization

2. Enhanced Partner Share Cap System

• Dynamic Cap Management: Added ability to set and remove partner share caps per token
• Cap Enforcement: Prevents minting shares beyond the set cap for specific tokens
• Tracking: Maintains separate tracking of shares minted by each partner token

3. Improved Shareholder Management

• Efficient Indexing: Added shareholder_index mapping for O(1) shareholder removal
• Better List Management: Optimized shareholder list operations
• New View Functions: Added get_shareholder_count() and get_shareholder_at_index()

4. Code Organization

• Modular Functions: Split large functions into smaller, focused internal functions
• Better Validation: Comprehensive parameter validation with clear error messages
• Event System: Enhanced event structure with proper categorization

🔧 New Features

Partner Share Cap System

The contract now supports setting share caps for specific partner tokens:

// Set a cap for USDT partner
set_partner_share_cap(usdt_address, 5000000_u256); // 5M shares cap

// Remove the cap
remove_partner_share_cap(usdt_address);

// Check current cap
let cap = get_partner_share_cap(usdt_address);

// Check shares minted by partner
let shares_minted = get_shares_minted_by_partner(usdt_address);

Enhanced Shareholder Management

// Get total number of shareholders
let count = get_shareholder_count();

// Get shareholder at specific index
let shareholder = get_shareholder_at_index(0);

🧪 Testing

⚠️ Important Note About Integration Tests

The file tests/test_partner_share_cap.cairo contains integration tests that demonstrate the partner share cap functionality. However, this is NOT the recommended approach for writing integration tests in Cairo/Starknet for the following reasons:

Issues with Current Test Approach:

1. Mixed Test Types: The file combines unit tests with integration test patterns
2. Complex Setup: Uses snforge_std cheat codes which are more suitable for unit testing
3. Hardcoded Values: Contains magic numbers and hardcoded addresses
4. Limited Scope: Tests only specific scenarios rather than comprehensive integration flows

Recommended Testing Strategy:

1. Unit Tests: Use snforge_std for testing individual functions in isolation
2. Integration Tests: Use starknet::testing for testing contract interactions
3. Separate Concerns: Keep unit tests and integration tests in separate files
4. Mock Contracts: Use proper mock contracts for external dependencies

Version Compatibility

⚠️ Important: These tests will run successfully on Cairo version 2.9.4 and compatible Starknet versions. The contract uses Cairo 2.x syntax and features that may not be compatible with older versions.

📁 Contract Structure

src/
├── lib.cairo                 # Main library file
├── BigIncGenesis.cairo       # Main contract implementation
├── ierc20.cairo             # ERC20 interface
└── mockerc20.cairo          # Mock ERC20 for testing

tests/
├── test_contract.cairo       # Basic contract tests
└── test_partner_share_cap.cairo  # Partner cap tests (see note above)

🚀 Key Functions

Core Functions

• mint_share(token_address) - Purchase shares with USDT/USDC
• transfer_share(to, amount) - Transfer shares to another address
• donate(token_address, amount) - Donate tokens to the contract

Owner Functions

• withdraw(token_address, amount) - Withdraw tokens from contract
• seize_shares(shareholder) - Seize shares from a shareholder
• set_partner_share_cap(token_address, cap) - Set partner share cap
• remove_partner_share_cap(token_address) - Remove partner share cap
• pause() / unpause() - Pause/unpause contract operations

View Functions

• get_available_shares() - Get remaining shares for sale
• get_shares(address) - Get shares owned by address
• get_partner_share_cap(token_address) - Get partner share cap
• get_shares_minted_by_partner(token_address) - Get shares minted by partner

🔒 Security Features

1. Access Control: Owner-only functions for administrative operations
2. Pausable: Contract can be paused in emergency situations
3. Input Validation: Comprehensive parameter validation
4. Partner Caps: Prevents excessive share minting by partners
5. Zero Address Checks: Prevents transfers to zero addresses

📊 Share Economics

• Total Valuation: $680,000 (680,000,000 with 6 decimals)
• Presale Valuation: $457,143 (457,143,000 with 6 decimals)
• Total Shares: 100,000,000
• Presale Shares: 21,000,000 (21%)
• Owner Shares: 18,000,000 (18%)
• Available Shares: 82,000,000 (82%)

🛠️ Development

Prerequisites

• Cairo 2.9.4+
• Scarb
• Starknet Foundry (for testing)

Build

scarb build

Test

snforge test 

[CollinsC1O]

Hello @Samuel1-ona Gm Gm!
So here is the thing, I've reviewed the refactored contract where you removed the OpenZeppelin dependencies, and I need to bring some serious security concerns to your attention. While I appreciate the cleaner code organization and better structure you've implemented, it definitely looks amazing, removing OpenZeppelin has introduced several critical vulnerabilities that could put user funds at risk.
Firstly, While your manual _assert_only_owner() implementation looks correct, you're essentially reinventing the wheel. OpenZeppelin's access control has been battle-tested across thousands of deployments, audited by multiple security firms and proven to handle edge cases we might not think of. Your implementation could have subtle bugs that only surface under specific conditions considering an implementation that has been thoroughly tested and proven we definitely will trust open zeppelin much more.
Secondly, your implementation’s got reentrancy attack vulnerability. This is the most dangerous issue. In your mint_share function, you're making external calls which deals with token transfer, without reentrancy protection, a malicious contract could; call mint_share again before the first transaction completes, potentially drain funds or manipulate share calculations and also exploit the time gap between state updates and external calls. The original openzepellin implementation had this covered with reentrancy_guard implementation, and I’m wondering why you took this out, can you enlighten me a bit.
Thirdly, your manual pause implementation is basic compared to OpenZeppelin's comprehensive state management. You might miss important edge cases or state transition scenarios that the OpenZeppelin component handles. This could create pausable mechanism gaps.
Now, I understand the desire to reduce dependencies and have cleaner code, but in smart contract development, security trumps everything else.

I will want you to go back to the OpenZeppelin implementation for Ownable, passable and most importantly the reentrancy implementation that open zeppelin provides. The security benefits far outweigh the dependency concerns. You can still keep your improved code organization and better documentation which I soo love, it looks amazing.
On a final note; Smart contracts are immutable once deployed, which we both know about. A single vulnerability could result in total loss of user funds which we soo don’t want at all. OpenZeppelin exists because they've solved these problems better than most of us could on our own.
Your code improvements (better organization, cleaner validation, improved shareholder management) are genuinely good but let's not sacrifice security for cleaner code. We can have both you know.

Let’s go back to using openzeppelin.
Thanks a lot for your help and implementation.

[Samuel1-ona]

@CollinsC1O Please, can you provide a PoC for the security concern?

[Samuel1-ona]

@CollinsC1O Please, can you provide a PoC for the security concern?

Regarding reentrancy, pause and others

[CollinsC1O]

@Samuel1-ona
Now the most severe issue lies in your mint_share function implementation where external token transfers occur without proper reentrancy protection.
Consider this attack scenario; a malicious attacker would deploy a contract that implements a receive function designed to re-enter the mint_share function. The attack contract would call the mint_share function with approved tokens, and when the contract attempts to transfer those tokens, the attacker's receive function would trigger, calling mint_share again with the same tokens before your contract updates its internal state. This recursive calling continues until the attacker has minted excessive shares relative to their actual token contribution. The fundamental issue is that your refactored contract performs external calls before updating internal state.
"OpenZeppelin's ReentrancyGuard" uses a simple but effective mutex pattern that prevents recursive calls to protect functions, completely eliminating this attack vector
Here is the thing, the OpenZeppelin's security implementations have undergone really extensive formal verifications, multiple security audits, and real-world testing across thousands of deployments, handling billions of dollars in value. Their implementations consider edge cases, gas optimization, and interaction patterns that we individually might overlook even as developers. The libraries represent collective wisdom from the entire Ethereum security community, making them significantly much more robust than custom implementations.
Your improved code organization are genuinely valuable contributions, but these enhancements can coexist with OpenZeppelin's battle-tested security implementations. The goal should be combining your structural improvements with proven security patterns rather than replacing established security mechanisms with custom alternatives

[CollinsC1O]

@Samuel1-ona please I would love we maintain the openzeppelin implementations, what do you think

[CollinsC1O]

Hello @Samuel1-ona you've been quite and not responded to my messages yet, please let me know from now till tomorrow if you're going to effect the requested changes. Thanks

[CollinsC1O]

revert to using the openzeppelin implementations for ownable, pausable and reentrancy guard.
when you're done with your changes pls remember to format your code, ensure it compiles ans all test passes

[CollinsC1O]

Hello @Samuel1-ona I will need a commit on the requested changes or a reply from now till tomorrow, that's if you are interested in implementing the changes I pointed out. Thanks a lot

[CollinsC1O]

Hello @Samuel1-ona can you use 'scarb fmt' to have your code formatted and resolve the merge conflict

[CollinsC1O]

Hello @Samuel1-ona Gm
can you please walk me on this so we can get it done with. we need this issue resolved already, it is stalling other issues.
please fmt your code, ensure it compiles and get all test passing. had someone make some changes so therefore the conflict but it should have flowed in with your implementation, so please can you get this done with before the end of today.
I need this before the end of today. thanks

[CollinsC1O]

Hello @Samuel1-ona I need a reply to know where thinks are at with you. I need this issue resolved

[Samuel1-ona]

I have done the fmt and everything. I think why the check is failing is because of the issue I pointed out earlier, version. As I said, most implementations are not documented yet on that version.

[CollinsC1O]

@Samuel1-ona Ok that's alright just get it to compile and all test passing, as it passed before it broke again. we've got other PR passing the build and test check. (#21) so check on what could be making yours fail and ensure its compiling and the test passing so I can merge it. That reminds me, please sync your fork with the main I will need to align the current implementation with yours

[jedstroke]

@Samuel1-ona, I have updated the Cairo workflow. The build-and-test passes, then the cairo-fmt fails because I suspect it is using the wrong workdir. Sync with main and make a trigger dummy commit, let's see this new workflow response.

Thank you.

[Samuel1-ona]

@CollinsC1O @jedstroke There is nothing breaking at the Cairo files. Why this is not passing is because of version 2.11.2. if you run the test code on your terminal, you will get expected gas error, which is coming from the version 2.11.1 but if you change your version to 2.9.4, all test cases will pass except the test file that your team member or someone wrote test_partner_share_cap.cairo which is not an integration test. So what you need to do is to reconfigure the contract version. At this moment it is all left to you guys to fix the version. Thank you.

[CollinsC1O]

Thanks a lot for the info @Samuel1-ona I will fix that. Thanks for your help

[jedstroke]

@CollinsC1O @jedstroke There is nothing breaking at the Cairo files. Why this is not passing is because of version 2.11.2. if you run the test code on your terminal, you will get expected gas error, which is coming from the version 2.11.1 but if you change your version to 2.9.4, all test cases will pass except the test file that your team member or someone wrote test_partner_share_cap.cairo which is not an integration test. So what you need to do is to reconfigure the contract version. At this moment it is all left to you guys to fix the version. Thank you.

A process breaks the Cairo fmt doesn't pass

[jedstroke]

@CollinsC1O @jedstroke There is nothing breaking at the Cairo files. Why this is not passing is because of version 2.11.2. if you run the test code on your terminal, you will get expected gas error, which is coming from the version 2.11.1 but if you change your version to 2.9.4, all test cases will pass except the test file that your team member or someone wrote test_partner_share_cap.cairo which is not an integration test. So what you need to do is to reconfigure the contract version. At this moment it is all left to you guys to fix the version. Thank you.

A process breaks the Cairo fmt doesn't pass

The build and test are passing

[jedstroke]

test_partner_share_cap.cairo

@Samuel1-ona But I am not seeing it fail

[jedstroke]

@CollinsC1O @jedstroke There is nothing breaking at the Cairo files. Why this is not passing is because of version 2.11.2. if you run the test code on your terminal, you will get expected gas error, which is coming from the version 2.11.1 but if you change your version to 2.9.4, all test cases will pass except the test file that your team member or someone wrote test_partner_share_cap.cairo which is not an integration test. So what you need to do is to reconfigure the contract version. At this moment it is all left to you guys to fix the version. Thank you.

@Samuel1-ona, just make the dummy commit push as requested. I am investigating the workflow error

[jedstroke]

@Samuel1-ona this is what I am investigating. Nobody said there is anything wrong with your implementation

[Samuel1-ona]

@CollinsC1O @jedstroke There is nothing breaking at the Cairo files. Why this is not passing is because of version 2.11.2. if you run the test code on your terminal, you will get expected gas error, which is coming from the version 2.11.1 but if you change your version to 2.9.4, all test cases will pass except the test file that your team member or someone wrote test_partner_share_cap.cairo which is not an integration test. So what you need to do is to reconfigure the contract version. At this moment it is all left to you guys to fix the version. Thank you.

A process breaks the Cairo fmt doesn't pass

The build and test are passing

Please share or make a video to show the test passing on your terminal for this particular test case test_partner_share_cap.cairo. And for the format, I did that the last time I made a commit. Thank you

[jedstroke]

@CollinsC1O @jedstroke There is nothing breaking at the Cairo files. Why this is not passing is because of version 2.11.2. if you run the test code on your terminal, you will get expected gas error, which is coming from the version 2.11.1 but if you change your version to 2.9.4, all test cases will pass except the test file that your team member or someone wrote test_partner_share_cap.cairo which is not an integration test. So what you need to do is to reconfigure the contract version. At this moment it is all left to you guys to fix the version. Thank you.

A process breaks the Cairo fmt doesn't pass

The build and test are passing

Please share or make a video to show the test passing on your terminal for this particular test case test_partner_share_cap.cairo. And for the format, I did that the last time I made a commit. Thank you

@Samuel1-ona Bro, how hard is it for you to push a dummy commit so I can validate the workflow?

[jedstroke]

test_partner_share_cap.cairo

@Samuel1-ona But I am not seeing it fail

As per your PR

[jedstroke]

@Samuel1-ona, the other contributor's test passed as per their PR, and their PR being merged doesn’t impact yours. If I don’t hear back from you within the next 1–4 hours about my request, I’ll go ahead without your PR to keep things moving. Thanks for your understanding — let me know if anything’s blocking you.