Skip to content
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.

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

Feature/predictoor iteration 2 #755

Merged
merged 10 commits into from
May 19, 2023
Merged

Conversation

alexcos20
Copy link
Member

@alexcos20 alexcos20 commented May 19, 2023

Fixes # .

Changes proposed in this PR:

  • add floatValue to submitTrueVal, for better accuracy tracking
  • allow owner to cancel an epoch by passing cancelRound = true in submitTrueVal
  • make sure that feeCollector gets fixed rate fees, slashed stakes, revenue per epoch if no predictoors (and not ERC20Deployer)
  • handle edge case when all stakers are wrong and will be slashed (feeCollector gets all stakes)

Tests:

  • Write test for edge case
  • Write test for "allow owner to cancel an epoch by passing cancelRound"

@alexcos20 alexcos20 requested a review from trizin May 19, 2023 05:36
@alexcos20 alexcos20 self-assigned this May 19, 2023
@alexcos20 alexcos20 requested a review from trentmc May 19, 2023 05:52
@alexcos20 alexcos20 marked this pull request as ready for review May 19, 2023 06:25
@trizin
Copy link
Contributor

trizin commented May 19, 2023

I'm having trouble understanding the purpose of floatValues as it doesn't seem to be utilized in the code.

@trizin
Copy link
Contributor

trizin commented May 19, 2023

I suggest renaming:

aggregatedPredictedValuesNumer -> roundSumStakesUp // Round number to the total sum of stakes placed on UP prediction
aggregatedPredictedValuesDenom -> roundSumStakes // Round number to the total sum of stakes placed

@alexcos20
Copy link
Member Author

I'm having trouble understanding the purpose of floatValues as it doesn't seem to be utilized in the code.

emit TruevalSubmitted(slot, trueValue,floatValue,epochStatus[slot]);

at https://github.com/oceanprotocol/contracts/blob/feature/predictoor_iteration_2/contracts/templates/ERC20Template3.sol#LL1138C9-L1138C77

@alexcos20
Copy link
Member Author

roundSumStakes 

done

@alexcos20 alexcos20 requested a review from trizin May 19, 2023 12:18
@trizin
Copy link
Contributor

trizin commented May 19, 2023

I'm having trouble understanding the purpose of floatValues as it doesn't seem to be utilized in the code.

emit TruevalSubmitted(slot, trueValue,floatValue,epochStatus[slot]);

at https://github.com/oceanprotocol/contracts/blob/feature/predictoor_iteration_2/contracts/templates/ERC20Template3.sol#LL1138C9-L1138C77

The mapping looks redundant. It's only used at:

floatValues[slot] = floatValue;

Considering it's not used anywhere else and we can query the events to get floatValue I suggest we remove the mapping. What do you think?

@alexcos20
Copy link
Member Author

I'm having trouble understanding the purpose of floatValues as it doesn't seem to be utilized in the code.

emit TruevalSubmitted(slot, trueValue,floatValue,epochStatus[slot]);

at https://github.com/oceanprotocol/contracts/blob/feature/predictoor_iteration_2/contracts/templates/ERC20Template3.sol#LL1138C9-L1138C77

The mapping looks redundant. It's only used at:

floatValues[slot] = floatValue;

Considering it's not used anywhere else and we can query the events to get floatValue I suggest we remove the mapping. What do you think?

yeap, removed

@alexcos20 alexcos20 requested a review from trizin May 19, 2023 12:43
Copy link
Contributor

@trizin trizin left a comment

Choose a reason for hiding this comment

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

LGTM

@alexcos20 alexcos20 merged commit 2a5bf67 into test/dt3 May 19, 2023
8 checks passed
@alexcos20 alexcos20 deleted the feature/predictoor_iteration_2 branch May 19, 2023 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants