enable blockchain and binary consensus to live in same branch about cbc-casper HOT 14 CLOSED

from @naterush

"make casper.py totally data structure agnostic and just have it deal w/ message generation and propagation. The plot tool can deal with everything else (pass it a view, and some information about the most recent round of msg propagation)."

from cbc-casper.

Agreed on utils. That's generally the way utils go in any repo...
"Don't know where to put this... I'll put it in utils!"
Three months later:
"Wow, wtf even is utils for"

from cbc-casper.

@naterush Can you fill in your current insights into this?

from cbc-casper.

@djrtwo gonna open specifically about making casper.py data structure agnostic :~)

from cbc-casper.

As per discussion today, here is an (incomplete) list of things that need to happen:

• Add a Message class that all message data structures inherit from. It handles the estimate, sender, and justification. The sequence_number can stay the same as well. Block and Bet will be data structures that inherit from this, and Block will add a height field. Block and Bet may choose to implement the is_in_blockchain, or maybe has_same_estimate
• As above, the View class will change to become more abstract. The get_new_messages function will remain the same. The justification function will likely be the same as well (pending a decision about some changes, see below). The estimate function will have to change for the estimator for each data structure, as well as the add_messages function. Finally, this abstract view will just keep track of messages and latest_messages - all other fields (children, last_finalized_block) will be left to the specific views for each data structure.
• Validator currently has to change as well, just due to the functions make_new_message and check_estimate_safety. make_new_message can easily be changed so the message class is passed to the validator when they are created, but check_estimate_safety is not so simple. One possible change here would be to have the view store a last_finalized_estimate (rather than a last_finalized_block). Then, we can continue to update the last_finalized_estimate in the case that we are talking about blockchain or binary (and note - we can have assert not utils.are_conflicting_estimates(new_finalized, old_finalized))
• utils isn't very well defined now - it's kinda an abstract collection of arbitrary functions. Will likely refactor + move the build_chain function elsewhere.
• test_lang needs to be refactored so it agnostic of data type. This might be a good time to think about using simulation runner if possible.
• Plotting need to change. The PlotTool itself is already agnostic ( pending PR: #79), but we need to build plotting tools to actually "create" the data to plot (what lines to draw what colors, what labels, etc). This could inherit from the PlotTool class and exist in the subdirectories for blockchain and binary respectively. Just have to implement the function that takes a view, and builds things accordingly (also would want to cache some things for efficiency).

Things left to think about:

1. Should we remove the last_finalized_block from the justification in blockchain data structures?

I think the answer should probably be yes. Though I'm sure it has some use, we don't use this anywhere currently -- and for now it's probably ok to remove.

from cbc-casper.

@djrtwo let me know if I missed anything big. I think this should be fairly comprehensive (though I'm sure we will run into more issues while implementing).

from cbc-casper.

I'd like the Validator to remain pretty thin and not care about what type of consensus they are a part of. I think it is totally reasonable for the View to generate the new message. Validator.make_new_message() can call View.new_message() or something. This keeps the validator from having to be consensus specific and will reduce the number of models that we have to make specific to each consensus protocol.

from cbc-casper.

Should probably call the Binary message class BinaryBet or something. Bet seems a bit too generic if we end up adding more protocols.

from cbc-casper.

Yeah, i just did a search on the Justification.last_finalized_block. Not used currently... was this for a potential efficiency gain around estimates or something?

from cbc-casper.

I'd like the Validator to remain pretty thin and not care about what type of consensus they are a part of. I think it is totally reasonable for the View to generate the new message. Validator.make_new_message() can call View.new_message() or something. This keeps the validator from having to be consensus specific and will reduce the number of models that we have to make specific to each consensus protocol.

I think we can get around this without pushing new message generation to the view. We can just pass in the message class to the validator, and they can use that message to create. If you check out the make_new_message function in the validator, it uses view.estimate and view.justification - and then the use of the block class is the only thing specific to the blockchain. So we can just pass in Block and/or Bet when creating the validator, and they use this to make new messages.

from cbc-casper.

Yeah, i just did a search on the Justification.last_finalized_block. Not used currently... was this for a potential efficiency gain around estimates or something?

I think so. Maybe for efficiency in knowing when to check an estimate for safety? Or for checking if a message is valid w/ a forkchoice starting from their last_finalized_estimate and checking the estimate (this makes sense as there used to be a children dict in the justification but it was not used and buggy, so I removed it).

from cbc-casper.

Yeah, that's reasonable. The validator is going to need to know about the protocol in question because they'll have to make the right View too.

Cool

from cbc-casper.

@djrtwo another possible thing we can do to further increase abstraction (and something I think we have to do for the testing language to be "abstractable") is to save self.last_finalized_estimate rather than last self.last_finalized_block.

In this case, we store the estimate (it's a block in the case of blockchain, and a bit in the case of binary) that was most recently finalized. I think this might make thinking about abstracting the testing language easier.

from cbc-casper.

Merged in this PR: #92

Yay!

from cbc-casper.