AOSSIE-Org / AOSSIE-Org/Agora-Blockchain

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

Aperta
#164 1 commento 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
JavaScript
Stelle
97
Fork
199
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

### 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

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.