AOSSIE-Org / AOSSIE-Org/Agora-Blockchain

BUG: Election.sol: in `removeCandidate()`, removing a candidate does not maintain the order of candidates.

Offen
#164 1 Kommentar 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
JavaScript
Sterne
97
Forks
199
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Issue Description ✍️

## 📌 Describe the Bug
If a candidate is removed, the function replaces the last candidate with the candidate to be removed and then pops the last candidate, hence the order of candidate is changed

i.e
```
candidates[_id] = candidates[candidates.length - 1];
candidate.pop();
```

## 🚨 Actual Behavior
For instance: if candidatesId array is [0,1,2,3,4,5], deleting the candidate 1 will result in the array: [0,5,2,3,4], which results in candidate name order changed.

## 🎯 Expected Behavior
considering above example, the resulted array should be [0,2,3,4,5] and the last candidate shouldn't be right after first candidate.

## 📷 Screenshot
### Before deletion:

![Image](https://github.com/user-attachments/assets/a3b3d322-6898-4ab3-97b8-feea75a79e77)

### After Deleting candidate with ID #1

![Image](https://github.com/user-attachments/assets/8c4cfe84-2fd6-4a76-9714-316c6abd581e)

## 💡 Suggestions
Threre are two fixes:

1. shift the elements to right places before popping (uses for loop and is very gas inefficient for large arrays):
```
function removeCandidate(uint _id) external onlyOwner electionStarted {
if (_id >= candidates.length) revert InvalidCandidateID();

// Shift all elements after the removed candidate one position to the left
for (uint i = _id; i < candidates.length - 1; i++) {
candidates[i] = candidates[i + 1];
}

// Remove the last element
candidates.pop();
}
```

2. Mark the candidate inactive instead of actually removing them:

```
struct Candidate {
uint candidateID;
string name;
string description;
bool isActive; // New field to track active status
}
```

and in function:

```
function removeCandidate(uint _id) external onlyOwner electionStarted {
if (_id >= candidates.length) revert InvalidCandidateID();

// Mark the candidate as inactive
candidates[_id].isActive = false;
}
```

and then in frontend code, just check the active status and display the names in order.

### Record

- [x] I have synced all my node versions as mentioned in the project
- [x] I am using the same version of npm as is the project
- [x] My current branch is in sync with the development branch
- [x] I want to work on this issue

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.