AOSSIE-Org / AOSSIE-Org/Agora-Blockchain

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

Abierto
#164 1 comentario 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
JavaScript
Estrellas
97
Forks
199
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

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

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.