set_finalizers host function (#1511) #1561
Conversation
Work in progress. - Implemented host function (not tested); - Finalizer set types; - Placeholders for fc::crypto::blslib::bls_public_key operators == != and <; - Stub notify_set_finalizers signal to be connected to chain_plugin, to reach chain_pacemaker
- Storing set_finalizer data at controller - chain_pacemaker retrievies set_finalizer data on get_finalizers() (not using it and not thread-safe yet) - Fix legacy span - Fix remove signal stub - Fix finalizer name to string description - Fix missing finalizer_set intrinsic whitelist upon feature activation
|
I'm thinking whether the structs in finalizer_set.hpp (copied from the producer schedule, mostly) are what we want:
|
|
@mschoenebeck is going to give me a hand to solve the operator== != < for bls_public_key, so we will be rolling that into this PR |
@fcecin @mschoenebeck Please use the C++20 spaceship operator I have a PR open to add them to the bls12 submodule, so after this is merged and the submodule tip updated in leap, you'll be able to add the comparisons with the same one liner. merged New PR to update submodule in leap. |
- Working ==, != and < operators in bls_public_key to enable testing this branch
|
I'll test the host function tomorrow (call it and see if it logs the same info at the pacemaker). After that basic test passes (I will notify) I think it will be good on my side for the final code reviews. (NOTE: I have no idea why the CI checks are failing right now or if that's actually relevant) |
Co-authored-by: Gregory Popovitch <[email protected]>
Co-authored-by: Gregory Popovitch <[email protected]>
- Tie up get_finalizer return value changes - Better operator< for bls_public_key
Co-authored-by: Gregory Popovitch <[email protected]>
Co-authored-by: Gregory Popovitch <[email protected]>
| ,public_key(public_key) | ||
| {} | ||
|
|
||
| std::string description; |
There was a problem hiding this comment.
This breaks compilation (don't know why) so leaving for later
| EOS_ASSERT( finalizers.size() > 0, wasm_execution_error, "Finalizer set cannot be empty" ); | ||
|
|
||
| std::set<fc::crypto::blslib::bls_public_key> unique_finalizer_keys; | ||
| #warning REVIEW: Is checking for unique finalizer descriptions at all relevant? |
- Fix breaking libtester w/ finalizer set fwd decl - Remove finalizer_authority::get_abi_variant - Fix init fthreshold in controller - Add const to controller get_finalizer_impl()
|
I'm thinking this is going to be tricky to test properly: to compile a contract with set_finalizers() in it, wouldn't an updated CDT be needed? But by reviewing the code, it either works or needs minor modifications, so I think it's OK for checking in and push testing for later. Unless someone has a better idea. The one question I have is whether the bls_public_key inside the finalizer_authority will correctly convert the base58 encoded string of the key at the contract level into the BLS library g1 object. I don't see why it wouldn't. |
IF: Add thread safety to chain_pacemaker access of chain state
This PR is not ready to be merged; this is not a request for a final review as the code is not complete. I created it to track progress on the set_finalizers branch and hold discussions. At some point this PR will have everything and we will merge.