Philipp #1
Philipp #1
Conversation
| | registeredProposer == True = [NoPenalty, Penalty Kicked] | ||
| | otherwise = [NoPenalty] | ||
|
|
||
| -- Submit on-chain report | ||
| actionsOnChainReport penalty = [NoReport, SubmitReport Validator, SubmitReport Builder, SubmitReport ValidatorKicked] | ||
| actionsOnChainReport penaltyValidator penaltyBuilder penaltyValidatorKicking (_,slot,proposerAddr,builderAddr,_,_) = [NoReport, SubmitReport report1, SubmitReport report2, SubmitReport report3] |
There was a problem hiding this comment.
Here three different record for three different types of penalty have been instantiated on the fly, right?
There was a problem hiding this comment.
Yes. The general idea is that the reporter before follows a certain logic, where exactly to identify a problem. That ends with a report. After that there is an explicit decision where this can be overruled. The idea being that in this way, we can in principle model different players doing different parts of the reporting. Not needed now but it would be a pain to add this later.
| preconditions | ||
| :: State -> ProposerAddr -> BuilderAddr -> SlotID -> Bool | ||
| preconditions state proposerAddr builderAddr slot | ||
| | checkReportInterval state slot == False = False |
There was a problem hiding this comment.
I find this syntax a bit difficult to parse, but maybe it's just me!
There was a problem hiding this comment.
Looks a bit dirty to me too. We're just checking that checkReportInterval state slot is True
There was a problem hiding this comment.
There's several instances of this. I don't know if @philipp-zahn prefers it that way but I can give it a quick pass and commit a cleaner version
There was a problem hiding this comment.
I think I tried to follow the logic in the docs. But if both of you feel this is confusing it is a sign we should do it differently. @dpl0a do you wanna take that over?
| | slotInPast == True | ||
| && registeredProposer == True | ||
| && missedPayment == True | ||
| && demand == True = [NoPenalty, Penalty Grieving] |
There was a problem hiding this comment.
These == True are redundant (there's several instances of that in the repo), IDK if it makes the code more readable for non-haskell people but let's keep that in mind
There was a problem hiding this comment.
Yes, my intention was to make it more readable. Also: the semantics of these bool conditions is not always clear, I fear. That's why I explicitly commented on them where I defined them.
| checkPayment :: State -> SlotID -> BuilderAddr -> Bool | ||
| checkPayment (State StateOnChain{..} StatePoNOnChain{..} _) slot builder = | ||
| let payment = M.lookup (slot,builder) paidInSlot | ||
| in if payment == Nothing |
There was a problem hiding this comment.
Lots of redundant if's in this module.
e.g. we can substitute this with isJust payment after importing Data.Maybe
There was a problem hiding this comment.
Yes that is a good point. I spend zero time on refactoring. @dpl0a can you take the lead on this?
| checkDemand :: State -> SlotID -> Bool | ||
| checkDemand (State StateOnChain{..} StatePoNOnChain{..} (_,auction)) slot = | ||
| let bids = auction M.! slot | ||
| in if bids == [] |
There was a problem hiding this comment.
Another example: here we can use bids /= []
There was a problem hiding this comment.
Same as above. @dpl0a again, wanna take over here?
Main reporter functionality; state definition; and parts of payout pool