Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

LLMOracleRegistry::unregister & LLMOracleCoordinator::finalizeValidation - the validator and generator allowances are correctly adjusted, but there is no function that allows to withdraw those fees / stake amounts

0xrs

Summary

Links:
https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/llm/LLMOracleRegistry.sol#L130
https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/llm/LLMOracleCoordinator.sol#L234
https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/llm/LLMOracleCoordinator.sol#L348
https://github.com/Cyfrin/2024-10-swan-dria/blob/c8686b199daadcef3161980022e12b66a5304f8e/contracts/llm/LLMOracleCoordinator.sol#L369

The finalizeValidation and unregister function correcly adjusts the allowance for each validator/generator, but there is no easy way for a validator/generator to retrieve the accumulated fees / stake amounts.

The docs state: "* Each oracle node can be thought of as an EOA that needs to register to the registry with some stake. The oracle can have two roles, generator or validator. They can register to both types if they would like to."

So, a validator/generator is an EOA and therfore it will be quite complicated to retrieve accumulated fees from the actual LLMOracleCoordinator/LLMOracleRegistry contract. A validator/generator would need to write a script using Ethers.js (or a similar library) that creates an instance of the fee token, connects with the corresponding validator/generator signer and calls the transferFrom method.

This, of course is unacceptable in terms of the user experience as most validators/generators would not be able to recover their fees.

Impact

The current contract behavior prevents validators/generators from accessing their accumulated fees / stake amounts, potentially resulting in a permanent loss of earned rewards / stake amounts

Tools Used

Manual Review

Recommendations

Remark: The following code sections only cover possible solutions to recover validator fees - the same code can be used to recover generator fees.

To ensure that validators/generators can access their fee tokens, a mechanism needs to be implemented that allows them to withdraw their fees. There are two approaches:

Direct Transfer to Validator: Transfer the fee immediately within the loop instead of modifying the allowance. This approach is straightforward but may be less flexible if you want to aggregate validator earnings over multiple validations:

- _increaseAllowance(validations[taskId][v_i].validator, task.validatorFee);
​
+ // Directly transfer the fee to the validator
+ address validator = validations[taskId][v_i].validator;
+ bool success = feeToken.transfer(validator, task.validatorFee);
+ require(success, "Fee transfer failed");

Withdraw Pattern: Track each validator's accumulated fees in the contract and allow them to withdraw their balance on demand. Implement a mapping to track each validator’s balance and a withdraw function:

mapping(address => uint256) public validatorBalances;
​
​
for (uint256 v_i = 0; v_i < task.parameters.numValidations; ++v_i) {
uint256 score = scores[v_i];
​
+ uint256 lowerBound = (_stddev <= _mean) ? _mean - _stddev : 0;
+ uint256 upperBound = _mean + _stddev;
​
if ((score >= lowerBound) && (score <= upperBound)) {
innerSum += score;
innerCount++;
- _increaseAllowance(validations[taskId][v_i].validator, task.validatorFee);
​
+ // Accumulate the fee in validatorBalances
+ address validator = validations[taskId][v_i].validator;
+ validatorBalances[validator] += task.validatorFee;
}
}

Add a withdraw function:

function withdrawFees() external {
uint256 amount = validatorBalances[msg.sender];
require(amount > 0, "No fees to withdraw");
​
validatorBalances[msg.sender] = 0;
​
bool success = feeToken.transfer(msg.sender, amount);
require(success, "Transfer failed");
}
​

For the LLMOracleRegistry::unregister functions, the following modifications are required:

Direct Transfer to the oracle: Transfer the stake amount in the unregister function instead of modifying the allowance:

function unregister(LLMOracleKind kind) public returns (uint256 amount) {
amount = registrations[msg.sender][kind];
​
// ensure the user is registered
if (amount == 0) {
revert NotRegistered(msg.sender);
}
​
// unregister the user
delete registrations[msg.sender][kind];
emit Unregistered(msg.sender, kind);
- token.approve(msg.sender, token.allowance(address(this), msg.sender) + amount);
+ bool success = token.transfer(msg.sender, amount);
+ require(success, "Transfer failed");
}

Withdraw Pattern: Track each oracle's accumulated stake amount in the contract and allow them to withdraw their balance on demand. Implement a mapping to track each oracle’s balance and a withdraw function:

mapping(address => uint256) public oracleBalances;
function unregister(LLMOracleKind kind) public returns (uint256 amount) {
amount = registrations[msg.sender][kind];
​
// ensure the user is registered
if (amount == 0) {
revert NotRegistered(msg.sender);
}
​
// unregister the user
delete registrations[msg.sender][kind];
emit Unregistered(msg.sender, kind);
- token.approve(msg.sender, token.allowance(address(this), msg.sender) + amount);
+ // Accumulate the stake amount in oracleBalances
+ oracleBalances[msg.sender] += amount;
}

Add a withdraw function:

function withdrawFees() external {
uint256 amount = oracleBalances[msg.sender];
require(amount > 0, "No stake amount to withdraw");
​
oracleBalances[msg.sender] = 0;
​
bool success = token.transfer(msg.sender, amount);
require(success, "Transfer failed");
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.