Skip to content

Minor gas optimizations in ElectionFactory #221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Esar1212 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Minor gas optimizations in ElectionFactory #221

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions blockchain/contracts/ElectionFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ contract ElectionFactory is CCIPReceiver {
}

uint public electionCount;
address public factoryOwner;
address public immutable factoryOwner;// factory owner does not need to be changed at any point of time. Using immutable reduces SLOAD cost.
address[] public openBasedElections;
// address[] public inviteBasedElections;

Expand Down Expand Up @@ -62,6 +62,7 @@ contract ElectionFactory is CCIPReceiver {
) external {
if (_candidates.length<2) revert InvalidCandidatesLength();
//add checks of time

address electionAddress = Clones.clone(electionGenerator);
address _ballot = ballotGenerator.generateBallot(
_ballotType,
Expand All @@ -77,17 +78,22 @@ contract ElectionFactory is CCIPReceiver {
msg.sender,
resultCalculator
);
electionCount++;
electionOwner[openBasedElections.length] = msg.sender;
unchecked{ //Skips overflow check as electionCount will never be that high.
electionCount++;
}
uint electionId = openBasedElections.length;
electionOwner[electionId] = msg.sender;
openBasedElections.push(address(election));
Comment on lines +81 to 86
Copy link

@coderabbitai coderabbitai bot Jan 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "*.sol" | head -20

Repository: AOSSIE-Org/Agora-Blockchain

Length of output: 1128

🏁 Script executed:

fd -t f -name "*.sol" | grep -E "(Election|election)" | head -20

Repository: AOSSIE-Org/Agora-Blockchain

Length of output: 301

🏁 Script executed:

rg -n "initialize\(" -t sol -C 2 | head -100

Repository: AOSSIE-Org/Agora-Blockchain

Length of output: 98

🏁 Script executed:

rg -n "electionCount" -t sol -C 2

Repository: AOSSIE-Org/Agora-Blockchain

Length of output: 98

🏁 Script executed:

rg -n "deleteElection|getOpenElections" -t sol -C 2

Repository: AOSSIE-Org/Agora-Blockchain

Length of output: 98

🏁 Script executed:

cat blockchain/contracts/Election.sol | head -50

Repository: AOSSIE-Org/Agora-Blockchain

Length of output: 1441

🏁 Script executed:

#!/bin/bash
# First, locate the files
echo "=== Finding Election-related files ==="
find . -type f -name "*.sol" | grep -i election

echo -e "\n=== Searching for initialize method signatures ==="
rg -n "function initialize" -t sol -C 2

echo -e "\n=== Searching for electionCount usage ==="
rg -n "electionCount" -t sol -C 2

echo -e "\n=== Searching for deleteElection ==="
rg -n "deleteElection" -t sol -C 3

echo -e "\n=== Searching for getOpenElections ==="
rg -n "getOpenElections" -t sol -C 3

Repository: AOSSIE-Org/Agora-Blockchain

Length of output: 490

🏁 Script executed:

#!/bin/bash
echo "=== Initialize method in Election.sol ==="
rg -n "function initialize" -A 5

echo -e "\n=== electionCount usage patterns ==="
rg -n "electionCount" -B 2 -A 2

echo -e "\n=== deleteElection implementation ==="
rg -n "deleteElection|function.*delete" -B 1 -A 5

echo -e "\n=== openBasedElections array operations ==="
rg -n "openBasedElections" -B 1 -A 1

echo -e "\n=== ID parameter passing to initialize ==="
rg -n "election.initialize|\.initialize" -B 2 -A 2

Repository: AOSSIE-Org/Agora-Blockchain

Length of output: 50386

Resolve ID inconsistency between electionCount and array indices.

The initialize() call receives electionCount as the election ID, but electionOwner is keyed by array indices (openBasedElections.length). While these align during creation, the swap-and-pop deletion in deleteElection() reorganizes array positions without updating stored election IDs, causing misalignment. If downstream code or users attempt deletion using a stored electionId, it will target the wrong mapping entry or array position. Use either a persistent counter for all ID assignments or refactor deletion to avoid array compaction (e.g., lazy deletion with a deleted flag).

🤖 Prompt for AI Agents 
In `@blockchain/contracts/ElectionFactory.sol` around lines 81 - 86, The code
mixes a persistent counter (electionCount) with array indices
(openBasedElections.length) which breaks after swap-and-pop in deleteElection();
fix by using a stable persistent ID for elections: assign id = electionCount,
set electionOwner[electionCount] = msg.sender, pass electionCount into
initialize(), push the election address into openBasedElections and also
maintain a mapping electionIdToIndex[electionCount] =
openBasedElections.length-1 (and update that mapping when you swap in
deleteElection()), increment electionCount afterwards; alternatively, if you
prefer no index mapping, change deleteElection() to perform lazy deletion (set a
deleted flag on the election struct) instead of swap-and-pop so stored IDs
remain valid (update functions referencing electionOwner, initialize,
deleteElection to use the chosen ID strategy).

}

function deleteElection(uint _electionId) external {
if (electionOwner[_electionId] != msg.sender) revert OnlyOwner();
uint lastElement = openBasedElections.length - 1;
address lastElection = openBasedElections[lastElement];
address lastOwner = electionOwner[lastElement];
if (_electionId != lastElement) {
openBasedElections[_electionId] = openBasedElections[lastElement];
electionOwner[_electionId] = electionOwner[lastElement];
openBasedElections[_electionId] = lastElection;
electionOwner[_electionId] = lastOwner;
}
openBasedElections.pop();
delete electionOwner[lastElement];
Expand Down