From c44e5d69aa408d98ce4bcca0d8d8f08a1026e5a4 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Mon, 28 Jun 2021 11:20:24 +0200 Subject: [PATCH] Decouple Staking and Election - Part 3: Signed Phase (#7910) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Base features and traits. * pallet and unsigned phase * add signed phase. * remove comments * Undo bad formattings. * some formatting cleanup. * Small self-cleanup. * Add todo * Make it all build * self-review * Some doc tests. * Some changes from other PR * Fix session test * Update bin/node/runtime/src/lib.rs Co-authored-by: Peter Goodspeed-Niklaus * Fix name. * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_election_provider_multi_phase --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/election-provider-multi-phase/src/weights.rs --template=./.maintain/frame-weight-template.hbs * typos and verbiage * no glob imports in signed.rs * meaningful generic type parameters for SignedSubmission * dedup feasibility check weight calculation * simplify/optimize fn insert_submission * tests: remove glob, cause to build without error * use sp_std::vec::Vec * maintain invariant within fn insert_submission * fix accidentally ordering the list backward * intentionally order the list in reverse * get rid of unused import * ensure signed submissions are cleared in early elect * finalize the signed phase when appropriate - ensure we don't leave storage lying around, even if elect called prematurely - test that proposition - disable the unsigned phase if a viable solution from the signed phase exists - ensure signed phase finalization weight is accounted for * resolve dispatch error todo * update assumptions in submit benchmark * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_election_provider_multi_phase --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/election-provider-multi-phase/src/weights.rs --template=./.maintain/frame-weight-template.hbs * line length * make a few more things pub * restore missing import * update ui test output * update tests from master branch * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_election_provider_multi_phase --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/election-provider-multi-phase/src/weights.rs --template=./.maintain/frame-weight-template.hbs * remove duplicate definitions * remove signed reward factor due to its attack potential * Update frame/election-provider-multi-phase/src/signed.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * remove SignedRewardMax; no longer necessary * compute the encoded size without actually encoding * remove unused PostInfo * pub use some stuff Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * ensure `pub use` things are in fact `pub` * add event information: was another solution ejected to make room * unconditionally run the unsigned phase even if signed was successful * remove dead test code * meaningful witness data name * use errors instead of defensive `unwrap_or_default` * get rid of a log message redundant with an event * saturating math Co-authored-by: Shawn Tabrizi * import Saturating * mv `fn submit` to end of call * add log line * Use a better data structure for SignedSubmissions instead of Vec (#8933) * Remove: (#8748) * `NetworkStatusSinks` * `sc_service::SpawnTasksParams::network_status_sinks` Also: * `sc_service::build_network()` does not return `network_status_sinks` * CI: fix simnet trigger (#8927) * CI: chore * CI: pin simnet version * More sc-service config reexports (#8887) * Reexport ExecutionStrategies and ExecutionStrategy * Reexport more of the network * Reexport the ExecutionStrategy as it's used within ExecutionStrategies * Fix check runtime CI (#8930) * Fix check_runtime.sh script * contracts: Remove confusing "Related Modules" doc * Bump parity-wasm and pwasm-utils to the newest versions everywhere (#8928) * BROKEN: convert SignedSubmissions to BoundedBTreeSet Eventually, once it works, this change should improve overall performance. However, in the meantime, the trait bounds aren't playing nicely, and this is turning into too much of a pain to handle right now as part of /#7910. We can take care of it later. * Simple `MaxBoundedLen` Implementations (#8793) * implement max_values + storages info * some formatting + doc * sudo sanity check * timestamp * assets (not working) * fix assets * impl for proxy * update balances * rename StoragesInfo -> PalletStorageInfo * merge both StorageInfoTrait and PalletStorageInfo I think it is more future proof. In the future some storage could make use of multiple prefix. Like one to store how much value has been inserted, etc... * Update frame/support/procedural/src/storage/parse.rs Co-authored-by: Peter Goodspeed-Niklaus * Update frame/support/procedural/src/storage/storage_struct.rs Co-authored-by: Peter Goodspeed-Niklaus * Fix max_size using hasher information hasher now expose `max_len` which allows to computes their maximum len. For hasher without concatenation, it is the size of the hash part, for hasher with concatenation, it is the size of the hash part + max encoded len of the key. * fix tests * fix ui tests * Move `MaxBoundedLen` into its own crate (#8814) * move MaxEncodedLen into its own crate * remove MaxEncodedLen impl from frame-support * add to assets and balances * try more fixes * fix compile Co-authored-by: Shawn Tabrizi * nits * fix compile * line width * fix max-values-macro merge * Add some derive, needed for test and other purpose * use weak bounded vec in some cases * Update lib.rs * move max-encoded-len crate * fix * remove app crypto for now * width * Revert "remove app crypto for now" This reverts commit 73623e9933d50648e0e7fe90b6171a8e45d7f5a2. * unused variable * more unused variables * more fixes * Add #[max_encoded_len_crate(...)] helper attribute The purpose of this attribute is to reduce the surface area of max_encoded_len changes. Crates deriving `MaxEncodedLen` do not need to add it to `Cargo.toml`; they can instead just do ```rust \#[derive(Encode, MaxEncodedLen)] \#[max_encoded_len_crate(frame_support::max_encoded_len)] struct Example; ``` * fix a ui test * use #[max_encoded_len_crate(...)] helper in app_crypto * remove max_encoded_len import where not necessary * update lockfile * fix ui test * ui * newline * fix merge * try fix ui again * Update max-encoded-len/derive/src/lib.rs Co-authored-by: Peter Goodspeed-Niklaus * extract generate_crate_access_2018 * Update lib.rs * compiler isnt smart enough Co-authored-by: thiolliere Co-authored-by: Peter Goodspeed-Niklaus Co-authored-by: Peter Goodspeed-Niklaus * remove duplicate Issued/Burned events (#8935) * weather -> whether (#8938) * make remote ext use batch ws-client (#8916) * make remote ext use batch ws-client * Add debug log for key length * better assertions * new sanity_checl * try and make it work with batch * update test * remove exctra uri * add missing at * remove unused rpc stuff * improve Co-authored-by: emostov <32168567+emostov@users.noreply.github.com> * Make `Schedule` fields public to allow for customization (#8924) * Make `Schedule` fields public for customization * Fix doc typo Co-authored-by: Andrew Jones Co-authored-by: Andrew Jones * Session key should be settable at genesis even for non-endowed accounts (#8942) * Session key should be settable at genesis even for non-endowed accounts * Docs * Migrate pallet-scored-pool to pallet attribute macro (#8825) * Migrate pallet-scored-pool to pallet attribute macro. * Remove dummy event. * Apply review suggestions. * Bump retain_mut from 0.1.2 to 0.1.3 (#8951) Bumps [retain_mut](https://github.com/upsuper/retain_mut) from 0.1.2 to 0.1.3. - [Release notes](https://github.com/upsuper/retain_mut/releases) - [Commits](https://github.com/upsuper/retain_mut/compare/v0.1.2...v0.1.3) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Use correct CreateInherentDataProviders impl for manual seal (#8852) * use correct CreateInherentDataProviders impl for manual seal * add babe inherent provider * move client into factory fn * Refactor code a little bit (#8932) * Optimize `next_storage_key` (#8956) * Optimize `next_storage_key` - Do not rely on recursion - Use an iterator over the overlay to not always call the same method * Fix bug * Add deserialize for TransactionValidityError in std. (#8961) * Add deserialize for TransactionValidityError in std. * Fix derives * Bump getrandom from 0.2.2 to 0.2.3 (#8952) Bumps [getrandom](https://github.com/rust-random/getrandom) from 0.2.2 to 0.2.3. - [Release notes](https://github.com/rust-random/getrandom/releases) - [Changelog](https://github.com/rust-random/getrandom/blob/master/CHANGELOG.md) - [Commits](https://github.com/rust-random/getrandom/compare/v0.2.2...v0.2.3) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Allow usage of path in construct_runtime! (#8801) * Allow usage of path in construct_runtime! * Fix whitespace * Fix whitespace * Make expand_runtime_metadata accept slice instead of Iterator * Include Call and Event in construct_runtime for testing * Migrate impl_outer_event to proc macro * Fix integrity_test_works * Update UI test expectations * Factor in module path while generating enum variant or fn names * Use ParseStream::lookahead for more helpful error messages * Remove generating outer_event_metadata * Ensure pallets with different paths but same last path segment can coexist * Remove unnecessary generated function * Migrate decl_outer_config to proc macro * Add default_filter test for expand_outer_origin * Allow crate, self and super keywords to appear in pallet path * Add UI test for specifying empty pallet paths in construct_runtime * Reduce cargo doc warnings (#8947) Co-authored-by: Bastian Köcher * Update wasmtime to 0.27 (#8913) * Update wasmtime to 0.27 A couple of notes: - Now we are fair about unsafeness of runtime creation via an compiled artifact. This change was prompted by the change in wasmtime which made `deserialize` rightfully unsafe. Now `CodeSupplyMode` was hidden and the `create_runtime` now takes the blob again and there is now a new fn for creating a runtime with a compiled artifact. - This is a big change for wasmtime. They switched to the modern backend for code generation. While this can bring performance improvements, it can also introduce some problems. In fact, 0.27 fixed a serious issue that could lead to sandbox escape. Hence we need a proper burn in. This would require a change to PVF validation host as well. * Filter regalloc logging * Spellling corrections (no code changes) (#8971) * Spelling corrections * As this might break let's do as a separate PR * Dependabot use correct label (#8973) * Inject hashed prefix for remote-ext (#8960) * Inject for remote-ext * Update utils/frame/remote-externalities/src/lib.rs Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com> * Update utils/frame/remote-externalities/src/lib.rs Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com> * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com> * Use `SpawnTaskHandle`s for spawning tasks in the tx pool (#8958) * Remove futures-diagnose * Use `SpawnTaskHandle`s for spawning tasks in the tx pool * Box the spawner * Fix tests * Use the testing task executor * Do not spend time on verifying the signatures before calling Runtime (#8980) * Revert "Use `SpawnTaskHandle`s for spawning tasks in the tx pool (#8958)" (#8983) This reverts commit bfef07c0d22ead3ab3c4e0e90ddf9b0e3537566e. * Uniques: An economically-secure basic-featured NFT pallet (#8813) * Uniques: An economically-secure basic-featured NFT pallet * force_transfer * freeze/thaw * team management * approvals * Fixes * force_asset_status * class_metadata * instance metadata * Fixes * use nmap * Fixes * class metadata has information field * Intiial mock/tests and a fix * Remove impl_non_fungibles * Docs * Update frame/uniques/src/lib.rs Co-authored-by: Shawn Tabrizi * Update frame/uniques/src/lib.rs Co-authored-by: Shawn Tabrizi * Update frame/uniques/src/lib.rs Co-authored-by: Shawn Tabrizi * Update frame/uniques/src/lib.rs Co-authored-by: Shawn Tabrizi * Reserve, don't transfer. * Fixes * Tests * Tests * refresh_deposit * Tests and proper handling of metdata destruction * test burn * Tests * Update impl_fungibles.rs * Initial benchmarking * benchmark * Fixes * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_uniques --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/uniques/src/weights.rs --template=./.maintain/frame-weight-template.hbs * Attributes * Attribute metadata * Fixes * Update frame/uniques/README.md * Docs * Docs * Docs * Simple metadata * Use BoundedVec * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_uniques --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/uniques/src/weights.rs --template=./.maintain/frame-weight-template.hbs * Update frame/uniques/src/lib.rs Co-authored-by: Lohann Paterno Coutinho Ferreira * Update frame/uniques/src/lib.rs Co-authored-by: Lohann Paterno Coutinho Ferreira * Update frame/uniques/src/lib.rs Co-authored-by: Lohann Paterno Coutinho Ferreira * Update frame/uniques/src/lib.rs Co-authored-by: Lohann Paterno Coutinho Ferreira * Update frame/uniques/src/lib.rs Co-authored-by: Lohann Paterno Coutinho Ferreira * Fixes * Update frame/uniques/README.md Co-authored-by: Alexander Popiak * Update frame/uniques/README.md Co-authored-by: Alexander Popiak * Update frame/uniques/README.md Co-authored-by: Alexander Popiak * Docs * Bump Co-authored-by: Shawn Tabrizi Co-authored-by: Parity Bot Co-authored-by: Lohann Paterno Coutinho Ferreira Co-authored-by: Alexander Popiak * Update WeakBoundedVec's remove and swap_remove (#8985) Co-authored-by: Boiethios * Convert another instance of Into impl to From in the macros (#8986) * Convert another instance of Into impl to From in the macros * Convert another location * also fix bounded vec (#8987) * fix most compiler errors Mostly the work so far has been in tracking down where precisely to insert appropriate trait bounds, and updating `fn insert_submission`. However, there's still a compiler error remaining: ``` error[E0275]: overflow evaluating the requirement `Compact<_>: Decode` | = help: consider adding a `#![recursion_limit="256"]` attribute to your crate (`pallet_election_provider_multi_phase`) = note: required because of the requirements on the impl of `Decode` for `Compact<_>` = note: 126 redundant requirements hidden = note: required because of the requirements on the impl of `Decode` for `Compact<_>` ``` Next up: figure out how we ended up with that recursive bound, and fix it. * extract type SignedSubmissionsOf Weirdly, we still encounter the recursive trait definition error here, despite removing the trait bounds. Something weird is happening. * impl Decode bounds on BoundedBTreeMap/Set on T, not predecessor Otherwise, Rust gets confused and decides that the trait bound is infinitely recursive. For that matter, it _still_ gets confused somehow and decides that the trait bound is infinitely recursive, but at least this should somewhat simplify the matter. * fix recursive trait bound problem * minor fixes * more little fixes * correct semantics for try_insert * more fixes * derive Ord for SolutionType * tests compile * fix most tests, rm unnecessary one * Transactionpool: Make `ready_at` return earlier (#8995) `ready_at` returns when we have processed the requested block. However, on startup we already have processed the best block and there are no transactions in the pool on startup anyway. So, we can set `updated_at` to the best block on startup. Besides that `ready_at` now returns early when there are no ready nor any future transactions in the pool. * Discard notifications if we have failed to parse handshake (#8806) * Migrate pallet-democracy to pallet attribute macro (#8824) * Migrate pallet-democracy to pallet attribute macro. * Metadata fix. * Trigger CI. * Add ecdsa::Pair::verify_prehashed() (#8996) * Add ecdsa::Pair::verify_prehashed() * turn verify_prehashed() into an associated function * add Signature::recover_prehashed() * Non-fungible token traits (#8993) * Non-fungible token traits * Docs * Fixes * Implement non-fungible trait for Uniques * Update frame/uniques/src/impl_nonfungibles.rs Co-authored-by: Shawn Tabrizi * Update frame/uniques/src/impl_nonfungibles.rs Co-authored-by: Shawn Tabrizi Co-authored-by: Shawn Tabrizi * Removes unused import (#9007) * Add Call Filter That Prevents Nested `batch_all` (#9009) * add filter preventing nested `batch_all` * more tests * fix test * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_utility --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/utility/src/weights.rs --template=./.maintain/frame-weight-template.hbs Co-authored-by: Parity Bot * Transaction pool: Ensure that we prune transactions properly (#8963) * Transaction pool: Ensure that we prune transactions properly There was a bug in the transaction pool that we didn't pruned transactions properly because we called `prune_known`, instead of `prune`. This bug was introduced by: https://github.com/paritytech/substrate/pull/4629 This is required to have stale extrinsics being removed properly, so that they don't fill up the tx pool. * Fix compilation * Fix benches * ... * Storage chain: Runtime module (#8624) * Transaction storage runtime module * WIP: Tests * Tests, benchmarks and docs * Made check_proof mandatory * Typo * Renamed a crate * Apply suggestions from code review Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Added weight for on_finalize * Fixed counter mutations * Reorganized tests * Fixed build * Update for the new inherent API * Reworked for the new inherents API * Apply suggestions from code review Co-authored-by: cheme Co-authored-by: Alexander Popiak Co-authored-by: Shawn Tabrizi * Store transactions in a Vec * Added FeeDestination * Get rid of constants * Fixed node runtime build * Fixed benches * Update frame/transaction-storage/src/lib.rs Co-authored-by: cheme Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: cheme Co-authored-by: Alexander Popiak Co-authored-by: Shawn Tabrizi * more useful error message (#9014) * Named reserve (#7778) * add NamedReservableCurrency * move currency related trait and types into a new file * implement NamedReservableCurrency * remove empty reserves * Update frame/support/src/traits.rs Co-authored-by: Shawn Tabrizi * fix build * bump year * add MaxReserves * repatriate_reserved_named should put reserved fund into named reserved * add tests * add some docs * fix warning * Update lib.rs * fix test * fix test * fix * fix * triggier CI * Move NamedReservableCurrency. * Use strongly bounded vec for reserves. * Fix test. * remove duplicated file * trigger CI * Make `ReserveIdentifier` assosicated type * add helpers * make ReserveIdentifier assosicated type * fix * update * trigger CI * Apply suggestions from code review Co-authored-by: Shawn Tabrizi * trigger CI * Apply suggestions from code review Co-authored-by: Shawn Tabrizi Co-authored-by: Gavin Wood Co-authored-by: Shaun Wang * update ss58 type to u16 (#8955) * Fixed build (#9021) * Bump parity-db (#9024) * consensus: handle justification sync for blocks authored locally (#8698) * consensus: add trait to control justification sync process * network: implement JustificationSyncLink for NetworkService * slots: handle justification sync in slot worker * babe: fix slot worker instantiation * aura: fix slot worker instantiation * pow: handle justification sync in miner * babe: fix tests * aura: fix tests * node: fix compilation * node-template: fix compilation * consensus: rename justification sync link parameter * aura: fix test compilation * consensus: slots: move JustificationSyncLink out of on_slot * arithmetic: fix PerThing pow (#9030) * arithmetic: add failing test for pow * arithmetic: fix PerThing::pow * Revert back to previous optimisations Co-authored-by: Gav Wood * Compact proof utilities in sp_trie. (#8574) * validation extension in sp_io * need paths * arc impl * missing host function in executor * io to pkdot * decode function. * encode primitive. * trailing tab * multiple patch * fix child trie logic * restore master versionning * bench compact proof size * trie-db 22.3 is needed * line width * split line * fixes for bench (additional root may not be needed as original issue was with empty proof). * revert compact from block size calculation. * New error type for compression. * Adding test (incomplete (failing)). Also lacking real proof checking (no good primitives in sp-trie crate). * There is currently no proof recording utility in sp_trie, removing test. * small test of child root in proof without a child proof. * remove empty test. * remove non compact proof size * Missing revert. * proof method to encode decode. * Don't inlucde nominaotrs that back no one in the snapshot. (#9017) * fix all_in_one test which had a logic error * use sp_std, not std * Periodically call `Peerset::alloc_slots` on all sets (#9025) * Periodically call alloc_slots on all slots * Add test * contracts: Add new `seal_call` that offers new features (#8909) * Add new `seal_call` that offers new features * Fix doc typo Co-authored-by: Michael Müller * Fix doc typos Co-authored-by: Michael Müller * Fix comment on assert * Update CHANGELOG.md Co-authored-by: Michael Müller * fix unreserve_all_named (#9042) * Delete legacy runtime metadata macros (#9043) * `rpc-http-threads` cli arg (#8890) * Add optional `rpc-http-threads` cli arg * Update `http::ServerBuilder`threads * allow inserting equal items into bounded map/set * refactor: only load one solution at a time This increases the database read load, because we read one solution at a time. On the other hand, it substantially decreases the overall memory load, because we _only_ read one solution at a time instead of reading all of them. * Emit `Bonded` event when rebonding (#9040) * Emit `Bonded` event when rebonding * fix borrow checker * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs Co-authored-by: Parity Bot * fix tests * Revert "Merge remote-tracking branch 'origin/master' into prgn-election-provider-multi-phase-bounded-btree-set-signed-submissions" This reverts commit de92b1e8e0e44a74c24e270d02b6e8e6a2c37032, reversing changes made to dae31f2018593b60dbf1d96ec96cdc35c374bb9e. * only derive debug when std * write after check * SignedSubmissions doesn't ever modify storage until .put() This makes a true check-before-write pattern possible. * REVERT ME: demo that Drop impl doesn't work * Revert "REVERT ME: demo that Drop impl doesn't work" This reverts commit 3317a4bb4de2e77d5a7fff2154552a81ec081763. * doc note about decode_len * rename get_submission, take_submission for clarity * add test which fails for current incorrect behavior * inline fn insert_submission This fixes a tricky check-before-write error, ensuring that we really only ever modify anything if we have in fact succeeded. Co-authored-by: Roman Proskuryakov Co-authored-by: Denis Pisarev Co-authored-by: MOZGIII Co-authored-by: Alexander Theißen Co-authored-by: Shawn Tabrizi Co-authored-by: thiolliere Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> Co-authored-by: Sebastian Müller Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: emostov <32168567+emostov@users.noreply.github.com> Co-authored-by: Andrew Jones Co-authored-by: Gavin Wood Co-authored-by: Shaun Wang Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Seun Lanlege Co-authored-by: Bastian Köcher Co-authored-by: Keith Yeung Co-authored-by: Squirrel Co-authored-by: Sergei Shulepov Co-authored-by: Ashley Co-authored-by: Parity Bot Co-authored-by: Lohann Paterno Coutinho Ferreira Co-authored-by: Alexander Popiak Co-authored-by: Boiethios Co-authored-by: Boiethios Co-authored-by: Pierre Krieger Co-authored-by: Andreas Doerr Co-authored-by: Dmitry Kashitsyn Co-authored-by: Arkadiy Paronyan Co-authored-by: cheme Co-authored-by: Andronik Ordian Co-authored-by: Xiliang Chen Co-authored-by: Gavin Wood Co-authored-by: Jakub Pánik Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> Co-authored-by: Michael Müller Co-authored-by: tgmichel * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_election_provider_multi_phase --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/election-provider-multi-phase/src/weights.rs --template=./.maintain/frame-weight-template.hbs * remove duplicate weight definitions injected by benchmark bot * check deletion overlay before getting * clarify non-conflict between delete, insert overlays * drain can be used wrong so is private * update take_submission docs * more drain improvements * more take_submission docs * debug assertion helps prove expectation is valid * doc on changing SignedMaxSubmissions * take_submission inner doc on system properties * Apply suggestions from code review Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com> Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * get SolutionOrSnapshotSize out of the loop Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com> * doc which items comprise `SignedSubmissions` * add doc about index as unique identifier * Add debug assertions to prove drain worked properly Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * replace take_submission with swap_out_submission * use a match to demonstrate all cases from signed_submissions.insert * refactor signed_submissions.insert return type * prettify test assertion Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * improve docs Co-authored-by: Guillaume Thiolliere * add tests that finalize_signed_phase is idempotent * add some debug assertions to guard against misuse of storage * log internal logic errors instead of panicing * don't store the reward with each signed submission The signed reward base can be treated as a constant. It can in principle change, but even if it's updated in the middle of an election, it's appropriate to use the current value for the winner. * emit Rewarded, Slashed events as appropriate Makes it easier to see who won/lost with signed submissions. * update docs * use a custom enum to be explicit about the outcome of insertion * remove outdated docs Co-authored-by: Peter Goodspeed-Niklaus Co-authored-by: Parity Benchmarking Bot Co-authored-by: Peter Goodspeed-Niklaus Co-authored-by: Shawn Tabrizi Co-authored-by: Roman Proskuryakov Co-authored-by: Denis Pisarev Co-authored-by: MOZGIII Co-authored-by: Alexander Theißen Co-authored-by: thiolliere Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> Co-authored-by: Sebastian Müller Co-authored-by: emostov <32168567+emostov@users.noreply.github.com> Co-authored-by: Andrew Jones Co-authored-by: Gavin Wood Co-authored-by: Shaun Wang Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Seun Lanlege Co-authored-by: Bastian Köcher Co-authored-by: Keith Yeung Co-authored-by: Squirrel Co-authored-by: Sergei Shulepov Co-authored-by: Ashley Co-authored-by: Lohann Paterno Coutinho Ferreira Co-authored-by: Alexander Popiak Co-authored-by: Boiethios Co-authored-by: Boiethios Co-authored-by: Pierre Krieger Co-authored-by: Andreas Doerr Co-authored-by: Dmitry Kashitsyn Co-authored-by: Arkadiy Paronyan Co-authored-by: cheme Co-authored-by: Andronik Ordian Co-authored-by: Xiliang Chen Co-authored-by: Gavin Wood Co-authored-by: Jakub Pánik Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> Co-authored-by: Michael Müller Co-authored-by: tgmichel --- bin/node/runtime/src/lib.rs | 21 +- .../src/benchmarking.rs | 105 +- .../src/helpers.rs | 8 +- .../election-provider-multi-phase/src/lib.rs | 272 +++++- .../election-provider-multi-phase/src/mock.rs | 56 +- .../src/signed.rs | 920 ++++++++++++++++++ .../src/unsigned.rs | 79 +- .../src/weights.rs | 107 +- .../support/src/storage/bounded_btree_map.rs | 64 +- .../support/src/storage/bounded_btree_set.rs | 62 +- primitives/npos-elections/compact/src/lib.rs | 2 +- 11 files changed, 1592 insertions(+), 104 deletions(-) create mode 100644 frame/election-provider-multi-phase/src/signed.rs diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 2ce19483e5539..fd7fd4213366f 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -73,6 +73,7 @@ use pallet_session::{historical as pallet_session_historical}; use sp_inherents::{InherentData, CheckInherentsResult}; use static_assertions::const_assert; use pallet_contracts::weights::WeightInfo; +use pallet_election_provider_multi_phase::FallbackStrategy; #[cfg(any(feature = "std", test))] pub use sp_runtime::BuildStorage; @@ -516,9 +517,14 @@ parameter_types! { pub const SignedPhase: u32 = EPOCH_DURATION_IN_BLOCKS / 4; pub const UnsignedPhase: u32 = EPOCH_DURATION_IN_BLOCKS / 4; - // fallback: no need to do on-chain phragmen initially. - pub const Fallback: pallet_election_provider_multi_phase::FallbackStrategy = - pallet_election_provider_multi_phase::FallbackStrategy::Nothing; + // signed config + pub const SignedMaxSubmissions: u32 = 10; + pub const SignedRewardBase: Balance = 1 * DOLLARS; + pub const SignedDepositBase: Balance = 1 * DOLLARS; + pub const SignedDepositByte: Balance = 1 * CENTS; + + // fallback: no on-chain fallback. + pub const Fallback: FallbackStrategy = FallbackStrategy::Nothing; pub SolutionImprovementThreshold: Perbill = Perbill::from_rational(1u32, 10_000); @@ -559,6 +565,14 @@ impl pallet_election_provider_multi_phase::Config for Runtime { type MinerMaxWeight = MinerMaxWeight; type MinerMaxLength = MinerMaxLength; type MinerTxPriority = MultiPhaseUnsignedPriority; + type SignedMaxSubmissions = SignedMaxSubmissions; + type SignedRewardBase = SignedRewardBase; + type SignedDepositBase = SignedDepositBase; + type SignedDepositByte = SignedDepositByte; + type SignedDepositWeight = (); + type SignedMaxWeight = MinerMaxWeight; + type SlashHandler = (); // burn slashes + type RewardHandler = (); // nothing to do upon rewards type DataProvider = Staking; type OnChainAccuracy = Perbill; type CompactSolution = NposCompactSolution16; @@ -1556,6 +1570,7 @@ impl_runtime_apis! { add_benchmark!(params, batches, pallet_uniques, Uniques); add_benchmark!(params, batches, pallet_utility, Utility); add_benchmark!(params, batches, pallet_vesting, Vesting); + add_benchmark!(params, batches, pallet_election_provider_multi_phase, ElectionProviderMultiPhase); if batches.is_empty() { return Err("Benchmark not found for this pallet.".into()) } Ok(batches) diff --git a/frame/election-provider-multi-phase/src/benchmarking.rs b/frame/election-provider-multi-phase/src/benchmarking.rs index 4eade8e184e75..7988163e98f65 100644 --- a/frame/election-provider-multi-phase/src/benchmarking.rs +++ b/frame/election-provider-multi-phase/src/benchmarking.rs @@ -19,7 +19,7 @@ use super::*; use crate::{Pallet as MultiPhase, unsigned::IndexAssignmentOf}; -use frame_benchmarking::impl_benchmark_test_suite; +use frame_benchmarking::{account, impl_benchmark_test_suite}; use frame_support::{assert_ok, traits::OnInitialize}; use frame_system::RawOrigin; use rand::{prelude::SliceRandom, rngs::SmallRng, SeedableRng}; @@ -57,7 +57,7 @@ fn solution_with_size( let targets: Vec = (0..size.targets).map(|i| frame_benchmarking::account("Targets", i, SEED)).collect(); - let mut rng = SmallRng::seed_from_u64(SEED as u64); + let mut rng = SmallRng::seed_from_u64(SEED.into()); // decide who are the winners. let winners = targets @@ -176,6 +176,39 @@ frame_benchmarking::benchmarks! { assert!(>::current_phase().is_unsigned()); } + finalize_signed_phase_accept_solution { + let receiver = account("receiver", 0, SEED); + let initial_balance = T::Currency::minimum_balance() * 10u32.into(); + T::Currency::make_free_balance_be(&receiver, initial_balance); + let ready: ReadySolution = Default::default(); + let deposit: BalanceOf = 10u32.into(); + let reward: BalanceOf = 20u32.into(); + + assert_ok!(T::Currency::reserve(&receiver, deposit)); + assert_eq!(T::Currency::free_balance(&receiver), initial_balance - 10u32.into()); + }: { + >::finalize_signed_phase_accept_solution(ready, &receiver, deposit, reward) + } verify { + assert_eq!(T::Currency::free_balance(&receiver), initial_balance + 20u32.into()); + assert_eq!(T::Currency::reserved_balance(&receiver), 0u32.into()); + } + + finalize_signed_phase_reject_solution { + let receiver = account("receiver", 0, SEED); + let initial_balance = T::Currency::minimum_balance().max(One::one()) * 10u32.into(); + let deposit: BalanceOf = 10u32.into(); + T::Currency::make_free_balance_be(&receiver, initial_balance); + assert_ok!(T::Currency::reserve(&receiver, deposit)); + + assert_eq!(T::Currency::free_balance(&receiver), initial_balance - 10u32.into()); + assert_eq!(T::Currency::reserved_balance(&receiver), 10u32.into()); + }: { + >::finalize_signed_phase_reject_solution(&receiver, deposit) + } verify { + assert_eq!(T::Currency::free_balance(&receiver), initial_balance - 10u32.into()); + assert_eq!(T::Currency::reserved_balance(&receiver), 0u32.into()); + } + on_initialize_open_unsigned_without_snapshot { // need to assume signed phase was open before >::on_initialize_open_signed().unwrap(); @@ -227,6 +260,38 @@ frame_benchmarking::benchmarks! { assert!(>::snapshot().is_some()); } + submit { + let c in 1 .. (T::SignedMaxSubmissions::get() - 1); + + // the solution will be worse than all of them meaning the score need to be checked against + // ~ log2(c) + let solution = RawSolution { + score: [(10_000_000u128 - 1).into(), 0, 0], + ..Default::default() + }; + + MultiPhase::::on_initialize_open_signed().expect("should be ok to start signed phase"); + >::put(1); + + let mut signed_submissions = SignedSubmissions::::get(); + for i in 0..c { + let solution = RawSolution { + score: [(10_000_000 + i).into(), 0, 0], + ..Default::default() + }; + let signed_submission = SignedSubmission { solution, ..Default::default() }; + signed_submissions.insert(signed_submission); + } + signed_submissions.put(); + + let caller = frame_benchmarking::whitelisted_caller(); + T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance() * 10u32.into()); + + }: _(RawOrigin::Signed(caller), solution, c) + verify { + assert!(>::signed_submissions().len() as u32 == c + 1); + } + submit_unsigned { // number of votes in snapshot. let v in (T::BenchmarkingConfig::VOTERS[0]) .. T::BenchmarkingConfig::VOTERS[1]; @@ -234,9 +299,12 @@ frame_benchmarking::benchmarks! { let t in (T::BenchmarkingConfig::TARGETS[0]) .. T::BenchmarkingConfig::TARGETS[1]; // number of assignments, i.e. compact.len(). This means the active nominators, thus must be // a subset of `v` component. - let a in (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1]; + let a in + (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1]; // number of desired targets. Must be a subset of `t` component. - let d in (T::BenchmarkingConfig::DESIRED_TARGETS[0]) .. T::BenchmarkingConfig::DESIRED_TARGETS[1]; + let d in + (T::BenchmarkingConfig::DESIRED_TARGETS[0]) .. + T::BenchmarkingConfig::DESIRED_TARGETS[1]; let witness = SolutionOrSnapshotSize { voters: v, targets: t }; let raw_solution = solution_with_size::(witness, a, d); @@ -249,7 +317,8 @@ frame_benchmarking::benchmarks! { let encoded_call = >::submit_unsigned(raw_solution.clone(), witness).encode(); }: { assert_ok!(>::submit_unsigned(RawOrigin::None.into(), raw_solution, witness)); - let _decoded_snap = as Decode>::decode(&mut &*encoded_snapshot).unwrap(); + let _decoded_snap = as Decode>::decode(&mut &*encoded_snapshot) + .unwrap(); let _decoded_call = as Decode>::decode(&mut &*encoded_call).unwrap(); } verify { assert!(>::queued_solution().is_some()); @@ -263,13 +332,17 @@ frame_benchmarking::benchmarks! { let t in (T::BenchmarkingConfig::TARGETS[0]) .. T::BenchmarkingConfig::TARGETS[1]; // number of assignments, i.e. compact.len(). This means the active nominators, thus must be // a subset of `v` component. - let a in (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1]; + let a in + (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1]; // number of desired targets. Must be a subset of `t` component. - let d in (T::BenchmarkingConfig::DESIRED_TARGETS[0]) .. T::BenchmarkingConfig::DESIRED_TARGETS[1]; + let d in + (T::BenchmarkingConfig::DESIRED_TARGETS[0]) .. + T::BenchmarkingConfig::DESIRED_TARGETS[1]; // Subtract this percentage from the actual encoded size let f in 0 .. 95; - // Compute a random solution, then work backwards to get the lists of voters, targets, and assignments + // Compute a random solution, then work backwards to get the lists of voters, targets, and + // assignments let witness = SolutionOrSnapshotSize { voters: v, targets: t }; let RawSolution { compact, .. } = solution_with_size::(witness, a, d); let RoundSnapshot { voters, targets } = MultiPhase::::snapshot().unwrap(); @@ -313,7 +386,11 @@ frame_benchmarking::benchmarks! { } verify { let compact = CompactOf::::try_from(index_assignments.as_slice()).unwrap(); let encoding = compact.encode(); - log!(trace, "encoded size prediction = {}", encoded_size_of(index_assignments.as_slice()).unwrap()); + log!( + trace, + "encoded size prediction = {}", + encoded_size_of(index_assignments.as_slice()).unwrap(), + ); log!(trace, "actual encoded size = {}", encoding.len()); assert!(encoding.len() <= desired_size); } @@ -326,9 +403,12 @@ frame_benchmarking::benchmarks! { let t in (T::BenchmarkingConfig::TARGETS[0]) .. T::BenchmarkingConfig::TARGETS[1]; // number of assignments, i.e. compact.len(). This means the active nominators, thus must be // a subset of `v` component. - let a in (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1]; + let a in + (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1]; // number of desired targets. Must be a subset of `t` component. - let d in (T::BenchmarkingConfig::DESIRED_TARGETS[0]) .. T::BenchmarkingConfig::DESIRED_TARGETS[1]; + let d in + (T::BenchmarkingConfig::DESIRED_TARGETS[0]) .. + T::BenchmarkingConfig::DESIRED_TARGETS[1]; let size = SolutionOrSnapshotSize { voters: v, targets: t }; let raw_solution = solution_with_size::(size, a, d); @@ -340,7 +420,8 @@ frame_benchmarking::benchmarks! { let encoded_snapshot = >::snapshot().unwrap().encode(); }: { assert_ok!(>::feasibility_check(raw_solution, ElectionCompute::Unsigned)); - let _decoded_snap = as Decode>::decode(&mut &*encoded_snapshot).unwrap(); + let _decoded_snap = as Decode>::decode(&mut &*encoded_snapshot) + .unwrap(); } } diff --git a/frame/election-provider-multi-phase/src/helpers.rs b/frame/election-provider-multi-phase/src/helpers.rs index bf5b360499cb4..46eeef0a6bf73 100644 --- a/frame/election-provider-multi-phase/src/helpers.rs +++ b/frame/election-provider-multi-phase/src/helpers.rs @@ -47,13 +47,13 @@ pub fn generate_voter_cache( cache } -/// Create a function the returns the index a voter in the snapshot. +/// Create a function that returns the index of a voter in the snapshot. /// /// The returning index type is the same as the one defined in `T::CompactSolution::Voter`. /// /// ## Warning /// -/// The snapshot must be the same is the one used to create `cache`. +/// Note that this will represent the snapshot data from which the `cache` is generated. pub fn voter_index_fn( cache: &BTreeMap, ) -> impl Fn(&T::AccountId) -> Option> + '_ { @@ -78,7 +78,7 @@ pub fn voter_index_fn_owned( /// /// ## Warning /// -/// The snapshot must be the same is the one used to create `cache`. +/// Note that this will represent the snapshot data from which the `cache` is generated. pub fn voter_index_fn_usize( cache: &BTreeMap, ) -> impl Fn(&T::AccountId) -> Option + '_ { @@ -103,7 +103,7 @@ pub fn voter_index_fn_linear( } } -/// Create a function the returns the index to a target in the snapshot. +/// Create a function that returns the index of a target in the snapshot. /// /// The returned index type is the same as the one defined in `T::CompactSolution::Target`. /// diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 2864ca518d068..45e04a757f0b3 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -231,7 +231,7 @@ use codec::{Decode, Encode}; use frame_support::{ dispatch::DispatchResultWithPostInfo, ensure, - traits::{Currency, Get, ReservableCurrency}, + traits::{Currency, Get, ReservableCurrency, OnUnbalanced}, weights::Weight, }; use frame_system::{ensure_none, offchain::SendTransactionTypes}; @@ -266,10 +266,14 @@ pub mod helpers; const LOG_TARGET: &'static str = "runtime::election-provider"; +pub mod signed; pub mod unsigned; pub mod weights; -/// The weight declaration of the pallet. +pub use signed::{ + BalanceOf, NegativeImbalanceOf, PositiveImbalanceOf, SignedSubmission, SignedSubmissionOf, + SignedSubmissions, SubmissionIndicesOf, +}; pub use weights::WeightInfo; /// The compact solution type used by this crate. @@ -411,7 +415,7 @@ impl Default for ElectionCompute { /// /// Such a solution should never become effective in anyway before being checked by the /// `Pallet::feasibility_check` -#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)] +#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, PartialOrd, Ord)] pub struct RawSolution { /// Compact election edges. pub compact: C, @@ -583,6 +587,44 @@ pub mod pallet { /// this value, based on [`WeightInfo::submit_unsigned`]. type MinerMaxWeight: Get; + /// Maximum number of signed submissions that can be queued. + /// + /// It is best to avoid adjusting this during an election, as it impacts downstream data + /// structures. In particular, `SignedSubmissionIndices` is bounded on this value. If you + /// update this value during an election, you _must_ ensure that + /// `SignedSubmissionIndices.len()` is less than or equal to the new value. Otherwise, + /// attempts to submit new solutions may cause a runtime panic. + #[pallet::constant] + type SignedMaxSubmissions: Get; + + /// Maximum weight of a signed solution. + /// + /// This should probably be similar to [`Config::MinerMaxWeight`]. + #[pallet::constant] + type SignedMaxWeight: Get; + + /// Base reward for a signed solution + #[pallet::constant] + type SignedRewardBase: Get>; + + /// Base deposit for a signed solution. + #[pallet::constant] + type SignedDepositBase: Get>; + + /// Per-byte deposit for a signed solution. + #[pallet::constant] + type SignedDepositByte: Get>; + + /// Per-weight deposit for a signed solution. + #[pallet::constant] + type SignedDepositWeight: Get>; + + /// Handler for the slashed deposits. + type SlashHandler: OnUnbalanced>; + + /// Handler for the rewards. + type RewardHandler: OnUnbalanced>; + /// Maximum length (bytes) that the mined solution should consume. /// /// The miner will ensure that the total length of the unsigned solution will not exceed @@ -599,6 +641,7 @@ pub mod pallet { + Eq + Clone + sp_std::fmt::Debug + + Ord + CompactSolution; /// Accuracy used for fallback on-chain election. @@ -656,11 +699,20 @@ pub mod pallet { Phase::Signed | Phase::Off if remaining <= unsigned_deadline && remaining > Zero::zero() => { - // Determine if followed by signed or not. + // our needs vary according to whether or not the unsigned phase follows a signed phase let (need_snapshot, enabled, signed_weight) = if current_phase == Phase::Signed { - // Followed by a signed phase: close the signed phase, no need for snapshot. - // TODO: proper weight https://github.com/paritytech/substrate/pull/7910. - (false, true, Weight::zero()) + // there was previously a signed phase: close the signed phase, no need for snapshot. + // + // Notes: + // + // - `Self::finalize_signed_phase()` also appears in `fn do_elect`. This is + // a guard against the case that `elect` is called prematurely. This adds + // a small amount of overhead, but that is unfortunately unavoidable. + let (_success, weight) = Self::finalize_signed_phase(); + // In the future we can consider disabling the unsigned phase if the signed + // phase completes successfully, but for now we're enabling it unconditionally + // as a defensive measure. + (false, true, weight) } else { // No signed phase: create a new snapshot, definitely `enable` the unsigned // phase. @@ -807,8 +859,12 @@ pub mod pallet { // Store the newly received solution. log!(info, "queued unsigned solution with score {:?}", ready.score); + let ejected_a_solution = >::exists(); >::put(ready); - Self::deposit_event(Event::SolutionStored(ElectionCompute::Unsigned)); + Self::deposit_event(Event::SolutionStored( + ElectionCompute::Unsigned, + ejected_a_solution, + )); Ok(None.into()) } @@ -828,6 +884,79 @@ pub mod pallet { Ok(()) } + /// Submit a solution for the signed phase. + /// + /// The dispatch origin fo this call must be __signed__. + /// + /// The solution is potentially queued, based on the claimed score and processed at the end + /// of the signed phase. + /// + /// A deposit is reserved and recorded for the solution. Based on the outcome, the solution + /// might be rewarded, slashed, or get all or a part of the deposit back. + /// + /// # + /// Queue size must be provided as witness data. + /// # + #[pallet::weight(T::WeightInfo::submit(*num_signed_submissions))] + pub fn submit( + origin: OriginFor, + solution: RawSolution>, + num_signed_submissions: u32, + ) -> DispatchResult { + let who = ensure_signed(origin)?; + + // ensure witness data is correct. + ensure!( + num_signed_submissions >= >::decode_len().unwrap_or_default() as u32, + Error::::SignedInvalidWitness, + ); + + // ensure solution is timely. + ensure!(Self::current_phase().is_signed(), Error::::PreDispatchEarlySubmission); + + // NOTE: this is the only case where having separate snapshot would have been better + // because could do just decode_len. But we can create abstractions to do this. + + // build size. Note: this is not needed for weight calc, thus not input. + // unlikely to ever return an error: if phase is signed, snapshot will exist. + let size = Self::snapshot_metadata().ok_or(Error::::MissingSnapshotMetadata)?; + + ensure!( + Self::feasibility_weight_of(&solution, size) < T::SignedMaxWeight::get(), + Error::::SignedTooMuchWeight, + ); + + // create the submission + let deposit = Self::deposit_for(&solution, size); + let submission = SignedSubmission { who: who.clone(), deposit, solution }; + + // insert the submission if the queue has space or it's better than the weakest + // eject the weakest if the queue was full + let mut signed_submissions = Self::signed_submissions(); + let maybe_removed = match signed_submissions.insert(submission) { + // it's an error if we failed to insert a submission: this indicates the queue was + // full but our solution had insufficient score to eject any solution + signed::InsertResult::NotInserted => return Err(Error::::SignedQueueFull.into()), + signed::InsertResult::Inserted => None, + signed::InsertResult::InsertedEjecting(weakest) => Some(weakest), + }; + + // collect deposit. Thereafter, the function cannot fail. + T::Currency::reserve(&who, deposit) + .map_err(|_| Error::::SignedCannotPayDeposit)?; + + let ejected_a_solution = maybe_removed.is_some(); + // if we had to remove the weakest solution, unreserve its deposit + if let Some(removed) = maybe_removed { + let _remainder = T::Currency::unreserve(&removed.who, removed.deposit); + debug_assert!(_remainder.is_zero()); + } + + signed_submissions.put(); + Self::deposit_event(Event::SolutionStored(ElectionCompute::Signed, ejected_a_solution)); + Ok(()) + } + /// Set a solution in the queue, to be handed out to the client of this pallet in the next /// call to `ElectionProvider::elect`. /// @@ -860,7 +989,9 @@ pub mod pallet { /// /// If the solution is signed, this means that it hasn't yet been processed. If the /// solution is unsigned, this means that it has also been processed. - SolutionStored(ElectionCompute), + /// + /// The `bool` is `true` when a previous solution was ejected to make room for this one. + SolutionStored(ElectionCompute, bool), /// The election has been finalized, with `Some` of the given computation, or else if the /// election failed, `None`. ElectionFinalized(Option), @@ -883,8 +1014,20 @@ pub mod pallet { PreDispatchWrongWinnerCount, /// Submission was too weak, score-wise. PreDispatchWeakSubmission, + /// The queue was full, and the solution was not better than any of the existing ones. + SignedQueueFull, + /// The origin failed to pay the deposit. + SignedCannotPayDeposit, + /// Witness data to dispatchable is invalid. + SignedInvalidWitness, + /// The signed submission consumes too much weight + SignedTooMuchWeight, /// OCW submitted solution for wrong round OcwCallWrongEra, + /// Snapshot metadata should exist but didn't. + MissingSnapshotMetadata, + /// `Self::insert_submission` returned an invalid index. + InvalidSubmissionIndex, /// The call is not allowed at this point. CallNotAllowed, } @@ -988,6 +1131,45 @@ pub mod pallet { #[pallet::getter(fn snapshot_metadata)] pub type SnapshotMetadata = StorageValue<_, SolutionOrSnapshotSize>; + // The following storage items collectively comprise `SignedSubmissions`, and should never be + // accessed independently. Instead, get `Self::signed_submissions()`, modify it as desired, and + // then do `signed_submissions.put()` when you're done with it. + + /// The next index to be assigned to an incoming signed submission. + /// + /// Every accepted submission is assigned a unique index; that index is bound to that particular + /// submission for the duration of the election. On election finalization, the next index is + /// reset to 0. + /// + /// We can't just use `SignedSubmissionIndices.len()`, because that's a bounded set; past its + /// capacity, it will simply saturate. We can't just iterate over `SignedSubmissionsMap`, + /// because iteration is slow. Instead, we store the value here. + #[pallet::storage] + pub(crate) type SignedSubmissionNextIndex = StorageValue<_, u32, ValueQuery>; + + /// A sorted, bounded set of `(score, index)`, where each `index` points to a value in + /// `SignedSubmissions`. + /// + /// We never need to process more than a single signed submission at a time. Signed submissions + /// can be quite large, so we're willing to pay the cost of multiple database accesses to access + /// them one at a time instead of reading and decoding all of them at once. + #[pallet::storage] + pub(crate) type SignedSubmissionIndices = + StorageValue<_, SubmissionIndicesOf, ValueQuery>; + + /// Unchecked, signed solutions. + /// + /// Together with `SubmissionIndices`, this stores a bounded set of `SignedSubmissions` while + /// allowing us to keep only a single one in memory at a time. + /// + /// Twox note: the key of the map is an auto-incrementing index which users cannot inspect or + /// affect; we shouldn't need a cryptographically secure hasher. + #[pallet::storage] + pub(crate) type SignedSubmissionsMap = + StorageMap<_, Twox64Concat, u32, SignedSubmissionOf, ValueQuery>; + + // `SignedSubmissions` items end here. + /// The minimum score that each 'untrusted' solution must attain in order to be considered /// feasible. /// @@ -1223,7 +1405,7 @@ impl Pallet { /// 3. Clear all snapshot data. fn rotate_round() { // Inc round. - >::mutate(|r| *r = *r + 1); + >::mutate(|r| *r += 1); // Phase is off now. >::put(Phase::Off); @@ -1242,6 +1424,13 @@ impl Pallet { } fn do_elect() -> Result<(Supports, Weight), ElectionError> { + // We have to unconditionally try finalizing the signed phase here. There are only two + // possibilities: + // + // - signed phase was open, in which case this is essential for correct functioning of the system + // - signed phase was complete or not started, in which case finalization is idempotent and + // inexpensive (1 read of an empty vector). + let (_, signed_finalize_weight) = Self::finalize_signed_phase(); >::take() .map_or_else( || match T::Fallback::get() { @@ -1261,7 +1450,7 @@ impl Pallet { if Self::round() != 1 { log!(info, "Finalized election round with compute {:?}.", compute); } - (supports, weight) + (supports, weight.saturating_add(signed_finalize_weight)) }) .map_err(|err| { Self::deposit_event(Event::ElectionFinalized(None)); @@ -1309,7 +1498,14 @@ mod feasibility_check { //! more. The best way to audit and review these tests is to try and come up with a solution //! that is invalid, but gets through the system as valid. - use super::{mock::*, *}; + use super::*; + use crate::{ + mock::{ + MultiPhase, Runtime, roll_to, TargetIndex, raw_solution, EpochLength, UnsignedPhase, + SignedPhase, VoterIndex, ExtBuilder, + }, + }; + use frame_support::assert_noop; const COMPUTE: ElectionCompute = ElectionCompute::OnChain; @@ -1476,16 +1672,24 @@ mod feasibility_check { #[cfg(test)] mod tests { - use super::{mock::*, Event, *}; + use super::*; + use crate::{ + Phase, + mock::{ + ExtBuilder, MultiPhase, Runtime, roll_to, MockWeightInfo, AccountId, TargetIndex, + Targets, multi_phase_events, System, SignedMaxSubmissions, + }, + }; use frame_election_provider_support::ElectionProvider; + use frame_support::{assert_noop, assert_ok}; use sp_npos_elections::Support; #[test] fn phase_rotation_works() { ExtBuilder::default().build_and_execute(|| { // 0 ------- 15 ------- 25 ------- 30 ------- ------- 45 ------- 55 ------- 60 - // | | | | - // Signed Unsigned Signed Unsigned + // | | | | | | + // Signed Unsigned Elect Signed Unsigned Elect assert_eq!(System::block_number(), 0); assert_eq!(MultiPhase::current_phase(), Phase::Off); @@ -1644,6 +1848,44 @@ mod tests { assert!(MultiPhase::snapshot_metadata().is_none()); assert!(MultiPhase::desired_targets().is_none()); assert!(MultiPhase::queued_solution().is_none()); + assert!(MultiPhase::signed_submissions().is_empty()); + }) + } + + #[test] + fn early_termination_with_submissions() { + // an early termination in the signed phase, with no queued solution. + ExtBuilder::default().build_and_execute(|| { + // signed phase started at block 15 and will end at 25. + roll_to(14); + assert_eq!(MultiPhase::current_phase(), Phase::Off); + + roll_to(15); + assert_eq!(multi_phase_events(), vec![Event::SignedPhaseStarted(1)]); + assert_eq!(MultiPhase::current_phase(), Phase::Signed); + assert_eq!(MultiPhase::round(), 1); + + // fill the queue with signed submissions + for s in 0..SignedMaxSubmissions::get() { + let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() }; + assert_ok!(MultiPhase::submit( + crate::mock::Origin::signed(99), + solution, + MultiPhase::signed_submissions().len() as u32 + )); + } + + // an unexpected call to elect. + roll_to(20); + assert!(MultiPhase::elect().is_ok()); + + // all storage items must be cleared. + assert_eq!(MultiPhase::round(), 2); + assert!(MultiPhase::snapshot().is_none()); + assert!(MultiPhase::snapshot_metadata().is_none()); + assert!(MultiPhase::desired_targets().is_none()); + assert!(MultiPhase::queued_solution().is_none()); + assert!(MultiPhase::signed_submissions().is_empty()); }) } diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index bd035aaf82969..8840e2b935d35 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -260,8 +260,13 @@ parameter_types! { pub static DesiredTargets: u32 = 2; pub static SignedPhase: u64 = 10; pub static UnsignedPhase: u64 = 5; - pub static MaxSignedSubmissions: u32 = 5; - + pub static SignedMaxSubmissions: u32 = 5; + pub static SignedDepositBase: Balance = 5; + pub static SignedDepositByte: Balance = 0; + pub static SignedDepositWeight: Balance = 0; + pub static SignedRewardBase: Balance = 7; + pub static SignedRewardMax: Balance = 10; + pub static SignedMaxWeight: Weight = BlockWeights::get().max_block; pub static MinerMaxIterations: u32 = 5; pub static MinerTxPriority: u64 = 100; pub static SolutionImprovementThreshold: Perbill = Perbill::zero(); @@ -304,6 +309,27 @@ impl multi_phase::weights::WeightInfo for DualMockWeightInfo { <() as multi_phase::weights::WeightInfo>::on_initialize_open_unsigned_without_snapshot() } } + fn finalize_signed_phase_accept_solution() -> Weight { + if MockWeightInfo::get() { + Zero::zero() + } else { + <() as multi_phase::weights::WeightInfo>::finalize_signed_phase_accept_solution() + } + } + fn finalize_signed_phase_reject_solution() -> Weight { + if MockWeightInfo::get() { + Zero::zero() + } else { + <() as multi_phase::weights::WeightInfo>::finalize_signed_phase_reject_solution() + } + } + fn submit(c: u32) -> Weight { + if MockWeightInfo::get() { + Zero::zero() + } else { + <() as multi_phase::weights::WeightInfo>::submit(c) + } + } fn elect_queued() -> Weight { if MockWeightInfo::get() { Zero::zero() @@ -342,6 +368,14 @@ impl crate::Config for Runtime { type MinerMaxWeight = MinerMaxWeight; type MinerMaxLength = MinerMaxLength; type MinerTxPriority = MinerTxPriority; + type SignedRewardBase = SignedRewardBase; + type SignedDepositBase = SignedDepositBase; + type SignedDepositByte = (); + type SignedDepositWeight = (); + type SignedMaxWeight = SignedMaxWeight; + type SignedMaxSubmissions = SignedMaxSubmissions; + type SlashHandler = (); + type RewardHandler = (); type DataProvider = StakingMock; type WeightInfo = DualMockWeightInfo; type BenchmarkingConfig = (); @@ -440,6 +474,20 @@ impl ExtBuilder { VOTERS.with(|v| v.borrow_mut().push((who, stake, targets))); self } + pub fn signed_max_submission(self, count: u32) -> Self { + ::set(count); + self + } + pub fn signed_deposit(self, base: u64, byte: u64, weight: u64) -> Self { + ::set(base); + ::set(byte); + ::set(weight); + self + } + pub fn signed_weight(self, weight: Weight) -> Self { + ::set(weight); + self + } pub fn build(self) -> sp_io::TestExternalities { sp_tracing::try_init_simple(); let mut storage = @@ -481,3 +529,7 @@ impl ExtBuilder { self.build().execute_with(test) } } + +pub(crate) fn balances(who: &u64) -> (u64, u64) { + (Balances::free_balance(who), Balances::reserved_balance(who)) +} diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs new file mode 100644 index 0000000000000..ba1123c1331ad --- /dev/null +++ b/frame/election-provider-multi-phase/src/signed.rs @@ -0,0 +1,920 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! The signed phase implementation. + +use crate::{ + CompactOf, Config, ElectionCompute, Pallet, RawSolution, ReadySolution, SolutionOrSnapshotSize, + Weight, WeightInfo, QueuedSolution, SignedSubmissionsMap, SignedSubmissionIndices, + SignedSubmissionNextIndex, +}; +use codec::{Encode, Decode, HasCompact}; +use frame_support::{ + storage::bounded_btree_map::BoundedBTreeMap, + traits::{Currency, Get, OnUnbalanced, ReservableCurrency}, + DebugNoBound, +}; +use sp_arithmetic::traits::SaturatedConversion; +use sp_npos_elections::{is_score_better, CompactSolution, ElectionScore}; +use sp_runtime::{ + RuntimeDebug, + traits::{Saturating, Zero}, +}; +use sp_std::{ + cmp::Ordering, + collections::{btree_map::BTreeMap, btree_set::BTreeSet}, + ops::Deref, +}; + +/// A raw, unchecked signed submission. +/// +/// This is just a wrapper around [`RawSolution`] and some additional info. +#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, Default)] +pub struct SignedSubmission { + /// Who submitted this solution. + pub who: AccountId, + /// The deposit reserved for storing this solution. + pub deposit: Balance, + /// The raw solution itself. + pub solution: RawSolution, +} + +impl Ord + for SignedSubmission +where + AccountId: Ord, + Balance: Ord + HasCompact, + CompactSolution: Ord, + RawSolution: Ord, +{ + fn cmp(&self, other: &Self) -> Ordering { + self.solution + .score + .cmp(&other.solution.score) + .then_with(|| self.solution.cmp(&other.solution)) + .then_with(|| self.deposit.cmp(&other.deposit)) + .then_with(|| self.who.cmp(&other.who)) + } +} + +impl PartialOrd + for SignedSubmission +where + AccountId: Ord, + Balance: Ord + HasCompact, + CompactSolution: Ord, + RawSolution: Ord, +{ + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +pub type BalanceOf = + <::Currency as Currency<::AccountId>>::Balance; +pub type PositiveImbalanceOf = <::Currency as Currency< + ::AccountId, +>>::PositiveImbalance; +pub type NegativeImbalanceOf = <::Currency as Currency< + ::AccountId, +>>::NegativeImbalance; +pub type SignedSubmissionOf = + SignedSubmission<::AccountId, BalanceOf, CompactOf>; + +pub type SubmissionIndicesOf = + BoundedBTreeMap::SignedMaxSubmissions>; + +/// Outcome of [`SignedSubmissions::insert`]. +pub enum InsertResult { + /// The submission was not inserted because the queue was full and the submission had + /// insufficient score to eject a prior solution from the queue. + NotInserted, + /// The submission was inserted successfully without ejecting a solution. + Inserted, + /// The submission was inserted successfully. As the queue was full, this operation ejected a + /// prior solution, contained in this variant. + InsertedEjecting(SignedSubmissionOf), +} + +/// Mask type which pretends to be a set of `SignedSubmissionOf`, while in fact delegating to the +/// actual implementations in `SignedSubmissionIndices`, `SignedSubmissionsMap`, and +/// `SignedSubmissionNextIndex`. +#[cfg_attr(feature = "std", derive(DebugNoBound))] +pub struct SignedSubmissions { + indices: SubmissionIndicesOf, + next_idx: u32, + insertion_overlay: BTreeMap>, + deletion_overlay: BTreeSet, +} + +impl SignedSubmissions { + /// Get the signed submissions from storage. + pub fn get() -> Self { + let submissions = SignedSubmissions { + indices: SignedSubmissionIndices::::get(), + next_idx: SignedSubmissionNextIndex::::get(), + insertion_overlay: BTreeMap::new(), + deletion_overlay: BTreeSet::new(), + }; + // validate that the stored state is sane + debug_assert!(submissions.indices.values().copied().max().map_or( + true, + |max_idx| submissions.next_idx > max_idx, + )); + submissions + } + + /// Put the signed submissions back into storage. + pub fn put(mut self) { + // validate that we're going to write only sane things to storage + debug_assert!(self.insertion_overlay.keys().copied().max().map_or( + true, + |max_idx| self.next_idx > max_idx, + )); + debug_assert!(self.indices.values().copied().max().map_or( + true, + |max_idx| self.next_idx > max_idx, + )); + + SignedSubmissionIndices::::put(self.indices); + SignedSubmissionNextIndex::::put(self.next_idx); + for key in self.deletion_overlay { + self.insertion_overlay.remove(&key); + SignedSubmissionsMap::::remove(key); + } + for (key, value) in self.insertion_overlay { + SignedSubmissionsMap::::insert(key, value); + } + } + + /// Get the submission at a particular index. + fn get_submission(&self, idx: u32) -> Option> { + if self.deletion_overlay.contains(&idx) { + // Note: can't actually remove the item from the insertion overlay (if present) + // because we don't want to use `&mut self` here. There may be some kind of + // `RefCell` optimization possible here in the future. + None + } else { + self.insertion_overlay + .get(&idx) + .cloned() + .or_else(|| SignedSubmissionsMap::::try_get(idx).ok()) + } + } + + /// Perform three operations: + /// + /// - Remove a submission (identified by score) + /// - Insert a new submission (identified by score and insertion index) + /// - Return the submission which was removed. + /// + /// Note: in the case that `weakest_score` is not present in `self.indices`, this will return + /// `None` without inserting the new submission and without further notice. + /// + /// Note: this does not enforce any ordering relation between the submission removed and that + /// inserted. + /// + /// Note: this doesn't insert into `insertion_overlay`, the optional new insertion must be + /// inserted into `insertion_overlay` to keep the variable `self` in a valid state. + fn swap_out_submission( + &mut self, + remove_score: ElectionScore, + insert: Option<(ElectionScore, u32)>, + ) -> Option> { + let remove_idx = self.indices.remove(&remove_score)?; + if let Some((insert_score, insert_idx)) = insert { + self.indices + .try_insert(insert_score, insert_idx) + .expect("just removed an item, we must be under capacity; qed"); + } + + self.insertion_overlay.remove(&remove_idx).or_else(|| { + (!self.deletion_overlay.contains(&remove_idx)).then(|| { + self.deletion_overlay.insert(remove_idx); + SignedSubmissionsMap::::try_get(remove_idx).ok() + }).flatten() + }) + } + + /// Iterate through the set of signed submissions in order of increasing score. + pub fn iter(&self) -> impl '_ + Iterator> { + self.indices.iter().filter_map(move |(_score, &idx)| { + let maybe_submission = self.get_submission(idx); + if maybe_submission.is_none() { + log!( + error, + "SignedSubmissions internal state is invalid (idx {}); \ + there is a logic error in code handling signed solution submissions", + idx, + ) + } + maybe_submission + }) + } + + /// Empty the set of signed submissions, returning an iterator of signed submissions in + /// arbitrary order. + /// + /// Note that if the iterator is dropped without consuming all elements, not all may be removed + /// from the underlying `SignedSubmissionsMap`, putting the storages into an invalid state. + /// + /// Note that, like `put`, this function consumes `Self` and modifies storage. + fn drain(mut self) -> impl Iterator> { + SignedSubmissionIndices::::kill(); + SignedSubmissionNextIndex::::kill(); + let insertion_overlay = sp_std::mem::take(&mut self.insertion_overlay); + SignedSubmissionsMap::::drain() + .filter(move |(k, _v)| !self.deletion_overlay.contains(k)) + .map(|(_k, v)| v) + .chain(insertion_overlay.into_iter().map(|(_k, v)| v)) + } + + /// Decode the length of the signed submissions without actually reading the entire struct into + /// memory. + /// + /// Note that if you hold an instance of `SignedSubmissions`, this function does _not_ + /// track its current length. This only decodes what is currently stored in memory. + pub fn decode_len() -> Option { + SignedSubmissionIndices::::decode_len() + } + + /// Insert a new signed submission into the set. + /// + /// In the event that the new submission is not better than the current weakest according + /// to `is_score_better`, we do not change anything. + pub fn insert( + &mut self, + submission: SignedSubmissionOf, + ) -> InsertResult { + // verify the expectation that we never reuse an index + debug_assert!(!self.indices.values().any(|&idx| idx == self.next_idx)); + + let weakest = match self.indices.try_insert(submission.solution.score, self.next_idx) { + Ok(Some(prev_idx)) => { + // a submission of equal score was already present in the set; + // no point editing the actual backing map as we know that the newer solution can't + // be better than the old. However, we do need to put the old value back. + self.indices + .try_insert(submission.solution.score, prev_idx) + .expect("didn't change the map size; qed"); + return InsertResult::NotInserted; + } + Ok(None) => { + // successfully inserted into the set; no need to take out weakest member + None + } + Err((insert_score, insert_idx)) => { + // could not insert into the set because it is full. + // note that we short-circuit return here in case the iteration produces `None`. + // If there wasn't a weakest entry to remove, then there must be a capacity of 0, + // which means that we can't meaningfully proceed. + let weakest_score = match self.indices.iter().next() { + None => return InsertResult::NotInserted, + Some((score, _)) => *score, + }; + let threshold = T::SolutionImprovementThreshold::get(); + + // if we haven't improved on the weakest score, don't change anything. + if !is_score_better(insert_score, weakest_score, threshold) { + return InsertResult::NotInserted; + } + + self.swap_out_submission(weakest_score, Some((insert_score, insert_idx))) + } + }; + + // we've taken out the weakest, so update the storage map and the next index + debug_assert!(!self.insertion_overlay.contains_key(&self.next_idx)); + self.insertion_overlay.insert(self.next_idx, submission); + debug_assert!(!self.deletion_overlay.contains(&self.next_idx)); + self.next_idx += 1; + match weakest { + Some(weakest) => InsertResult::InsertedEjecting(weakest), + None => InsertResult::Inserted, + } + } + + /// Remove the signed submission with the highest score from the set. + pub fn pop_last(&mut self) -> Option> { + let (score, _) = self.indices.iter().rev().next()?; + // deref in advance to prevent mutable-immutable borrow conflict + let score = *score; + self.swap_out_submission(score, None) + } +} + +impl Deref for SignedSubmissions { + type Target = SubmissionIndicesOf; + + fn deref(&self) -> &Self::Target { + &self.indices + } +} + +impl Pallet { + /// `Self` accessor for `SignedSubmission`. + pub fn signed_submissions() -> SignedSubmissions { + SignedSubmissions::::get() + } + + /// Finish the signed phase. Process the signed submissions from best to worse until a valid one + /// is found, rewarding the best one and slashing the invalid ones along the way. + /// + /// Returns true if we have a good solution in the signed phase. + /// + /// This drains the [`SignedSubmissions`], potentially storing the best valid one in + /// [`QueuedSolution`]. + pub fn finalize_signed_phase() -> (bool, Weight) { + let mut all_submissions = Self::signed_submissions(); + let mut found_solution = false; + let mut weight = T::DbWeight::get().reads(1); + + let SolutionOrSnapshotSize { voters, targets } = + Self::snapshot_metadata().unwrap_or_default(); + + let reward = T::SignedRewardBase::get(); + + while let Some(best) = all_submissions.pop_last() { + let SignedSubmission { solution, who, deposit} = best; + let active_voters = solution.compact.voter_count() as u32; + let feasibility_weight = { + // defensive only: at the end of signed phase, snapshot will exits. + let desired_targets = Self::desired_targets().unwrap_or_default(); + T::WeightInfo::feasibility_check( + voters, + targets, + active_voters, + desired_targets, + ) + }; + // the feasibility check itself has some weight + weight = weight.saturating_add(feasibility_weight); + match Self::feasibility_check(solution, ElectionCompute::Signed) { + Ok(ready_solution) => { + Self::finalize_signed_phase_accept_solution( + ready_solution, + &who, + deposit, + reward, + ); + found_solution = true; + + weight = weight + .saturating_add(T::WeightInfo::finalize_signed_phase_accept_solution()); + break; + } + Err(_) => { + Self::finalize_signed_phase_reject_solution(&who, deposit); + weight = weight + .saturating_add(T::WeightInfo::finalize_signed_phase_reject_solution()); + } + } + } + + // Any unprocessed solution is pointless to even consider. Feasible or malicious, + // they didn't end up being used. Unreserve the bonds. + let discarded = all_submissions.len(); + for SignedSubmission { who, deposit, .. } in all_submissions.drain() { + let _remaining = T::Currency::unreserve(&who, deposit); + weight = weight.saturating_add(T::DbWeight::get().writes(1)); + debug_assert!(_remaining.is_zero()); + } + + debug_assert!(!SignedSubmissionIndices::::exists()); + debug_assert!(!SignedSubmissionNextIndex::::exists()); + debug_assert!(SignedSubmissionsMap::::iter().next().is_none()); + + log!(debug, "closed signed phase, found solution? {}, discarded {}", found_solution, discarded); + (found_solution, weight) + } + + /// Helper function for the case where a solution is accepted in the signed phase. + /// + /// Extracted to facilitate with weight calculation. + /// + /// Infallible + pub fn finalize_signed_phase_accept_solution( + ready_solution: ReadySolution, + who: &T::AccountId, + deposit: BalanceOf, + reward: BalanceOf, + ) { + // write this ready solution. + >::put(ready_solution); + + // emit reward event + Self::deposit_event(crate::Event::Rewarded(who.clone())); + + // unreserve deposit. + let _remaining = T::Currency::unreserve(who, deposit); + debug_assert!(_remaining.is_zero()); + + // Reward. + let positive_imbalance = T::Currency::deposit_creating(who, reward); + T::RewardHandler::on_unbalanced(positive_imbalance); + } + + /// Helper function for the case where a solution is accepted in the rejected phase. + /// + /// Extracted to facilitate with weight calculation. + /// + /// Infallible + pub fn finalize_signed_phase_reject_solution(who: &T::AccountId, deposit: BalanceOf) { + Self::deposit_event(crate::Event::Slashed(who.clone())); + let (negative_imbalance, _remaining) = T::Currency::slash_reserved(who, deposit); + debug_assert!(_remaining.is_zero()); + T::SlashHandler::on_unbalanced(negative_imbalance); + } + + /// The feasibility weight of the given raw solution. + pub fn feasibility_weight_of( + solution: &RawSolution>, + size: SolutionOrSnapshotSize, + ) -> Weight { + T::WeightInfo::feasibility_check( + size.voters, + size.targets, + solution.compact.voter_count() as u32, + solution.compact.unique_targets().len() as u32, + ) + } + + /// Collect a sufficient deposit to store this solution. + /// + /// The deposit is composed of 3 main elements: + /// + /// 1. base deposit, fixed for all submissions. + /// 2. a per-byte deposit, for renting the state usage. + /// 3. a per-weight deposit, for the potential weight usage in an upcoming on_initialize + pub fn deposit_for( + solution: &RawSolution>, + size: SolutionOrSnapshotSize, + ) -> BalanceOf { + let encoded_len: u32 = solution.encoded_size().saturated_into(); + let encoded_len: BalanceOf = encoded_len.into(); + let feasibility_weight = Self::feasibility_weight_of(solution, size); + + let len_deposit = T::SignedDepositByte::get().saturating_mul(encoded_len); + let weight_deposit = T::SignedDepositWeight::get().saturating_mul(feasibility_weight.saturated_into()); + + T::SignedDepositBase::get().saturating_add(len_deposit).saturating_add(weight_deposit) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + Phase, Error, + mock::{ + balances, ExtBuilder, MultiPhase, Origin, raw_solution, roll_to, Runtime, + SignedMaxSubmissions, SignedMaxWeight, + }, + }; + use frame_support::{dispatch::DispatchResult, assert_noop, assert_storage_noop, assert_ok}; + + fn submit_with_witness( + origin: Origin, + solution: RawSolution>, + ) -> DispatchResult { + MultiPhase::submit(origin, solution, MultiPhase::signed_submissions().len() as u32) + } + + #[test] + fn cannot_submit_too_early() { + ExtBuilder::default().build_and_execute(|| { + roll_to(2); + assert_eq!(MultiPhase::current_phase(), Phase::Off); + + // create a temp snapshot only for this test. + MultiPhase::create_snapshot().unwrap(); + let solution = raw_solution(); + + assert_noop!( + submit_with_witness(Origin::signed(10), solution), + Error::::PreDispatchEarlySubmission, + ); + }) + } + + #[test] + fn wrong_witness_fails() { + ExtBuilder::default().build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + let solution = raw_solution(); + // submit this once correctly + assert_ok!(submit_with_witness(Origin::signed(99), solution.clone())); + assert_eq!(MultiPhase::signed_submissions().len(), 1); + + // now try and cheat by passing a lower queue length + assert_noop!( + MultiPhase::submit(Origin::signed(99), solution, 0), + Error::::SignedInvalidWitness, + ); + }) + } + + #[test] + fn should_pay_deposit() { + ExtBuilder::default().build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + let solution = raw_solution(); + assert_eq!(balances(&99), (100, 0)); + + assert_ok!(submit_with_witness(Origin::signed(99), solution)); + + assert_eq!(balances(&99), (95, 5)); + assert_eq!(MultiPhase::signed_submissions().iter().next().unwrap().deposit, 5); + }) + } + + #[test] + fn good_solution_is_rewarded() { + ExtBuilder::default().build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + let solution = raw_solution(); + assert_eq!(balances(&99), (100, 0)); + + assert_ok!(submit_with_witness(Origin::signed(99), solution)); + assert_eq!(balances(&99), (95, 5)); + + assert!(MultiPhase::finalize_signed_phase().0); + assert_eq!(balances(&99), (100 + 7, 0)); + }) + } + + #[test] + fn bad_solution_is_slashed() { + ExtBuilder::default().build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + let mut solution = raw_solution(); + assert_eq!(balances(&99), (100, 0)); + + // make the solution invalid. + solution.score[0] += 1; + + assert_ok!(submit_with_witness(Origin::signed(99), solution)); + assert_eq!(balances(&99), (95, 5)); + + // no good solution was stored. + assert!(!MultiPhase::finalize_signed_phase().0); + // and the bond is gone. + assert_eq!(balances(&99), (95, 0)); + }) + } + + #[test] + fn suppressed_solution_gets_bond_back() { + ExtBuilder::default().build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + let mut solution = raw_solution(); + assert_eq!(balances(&99), (100, 0)); + assert_eq!(balances(&999), (100, 0)); + + // submit as correct. + assert_ok!(submit_with_witness(Origin::signed(99), solution.clone())); + + // make the solution invalid and weaker. + solution.score[0] -= 1; + assert_ok!(submit_with_witness(Origin::signed(999), solution)); + assert_eq!(balances(&99), (95, 5)); + assert_eq!(balances(&999), (95, 5)); + + // _some_ good solution was stored. + assert!(MultiPhase::finalize_signed_phase().0); + + // 99 is rewarded. + assert_eq!(balances(&99), (100 + 7, 0)); + // 999 gets everything back. + assert_eq!(balances(&999), (100, 0)); + }) + } + + #[test] + fn cannot_submit_worse_with_full_queue() { + ExtBuilder::default().build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + for s in 0..SignedMaxSubmissions::get() { + // score is always getting better + let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() }; + assert_ok!(submit_with_witness(Origin::signed(99), solution)); + } + + + // weaker. + let solution = RawSolution { score: [4, 0, 0], ..Default::default() }; + + assert_noop!( + submit_with_witness(Origin::signed(99), solution), + Error::::SignedQueueFull, + ); + }) + } + + #[test] + fn weakest_is_removed_if_better_provided() { + ExtBuilder::default().build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + for s in 0..SignedMaxSubmissions::get() { + // score is always getting better + let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() }; + assert_ok!(submit_with_witness(Origin::signed(99), solution)); + } + + assert_eq!( + MultiPhase::signed_submissions() + .iter() + .map(|s| s.solution.score[0]) + .collect::>(), + vec![5, 6, 7, 8, 9] + ); + + // better. + let solution = RawSolution { score: [20, 0, 0], ..Default::default() }; + assert_ok!(submit_with_witness(Origin::signed(99), solution)); + + // the one with score 5 was rejected, the new one inserted. + assert_eq!( + MultiPhase::signed_submissions() + .iter() + .map(|s| s.solution.score[0]) + .collect::>(), + vec![6, 7, 8, 9, 20] + ); + }) + } + + #[test] + fn replace_weakest_works() { + ExtBuilder::default().build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + for s in 1..SignedMaxSubmissions::get() { + // score is always getting better + let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() }; + assert_ok!(submit_with_witness(Origin::signed(99), solution)); + } + + let solution = RawSolution { score: [4, 0, 0], ..Default::default() }; + assert_ok!(submit_with_witness(Origin::signed(99), solution)); + + assert_eq!( + MultiPhase::signed_submissions() + .iter() + .map(|s| s.solution.score[0]) + .collect::>(), + vec![4, 6, 7, 8, 9], + ); + + // better. + let solution = RawSolution { score: [5, 0, 0], ..Default::default() }; + assert_ok!(submit_with_witness(Origin::signed(99), solution)); + + // the one with score 5 was rejected, the new one inserted. + assert_eq!( + MultiPhase::signed_submissions() + .iter() + .map(|s| s.solution.score[0]) + .collect::>(), + vec![5, 6, 7, 8, 9], + ); + }) + } + + #[test] + fn early_ejected_solution_gets_bond_back() { + ExtBuilder::default().signed_deposit(2, 0, 0).build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + for s in 0..SignedMaxSubmissions::get() { + // score is always getting better + let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() }; + assert_ok!(submit_with_witness(Origin::signed(99), solution)); + } + + assert_eq!(balances(&99).1, 2 * 5); + assert_eq!(balances(&999).1, 0); + + // better. + let solution = RawSolution { score: [20, 0, 0], ..Default::default() }; + assert_ok!(submit_with_witness(Origin::signed(999), solution)); + + // got one bond back. + assert_eq!(balances(&99).1, 2 * 4); + assert_eq!(balances(&999).1, 2); + }) + } + + #[test] + fn equally_good_solution_is_not_accepted() { + ExtBuilder::default().signed_max_submission(3).build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + for i in 0..SignedMaxSubmissions::get() { + let solution = RawSolution { score: [(5 + i).into(), 0, 0], ..Default::default() }; + assert_ok!(submit_with_witness(Origin::signed(99), solution)); + } + assert_eq!( + MultiPhase::signed_submissions() + .iter() + .map(|s| s.solution.score[0]) + .collect::>(), + vec![5, 6, 7] + ); + + // 5 is not accepted. This will only cause processing with no benefit. + let solution = RawSolution { score: [5, 0, 0], ..Default::default() }; + assert_noop!( + submit_with_witness(Origin::signed(99), solution), + Error::::SignedQueueFull, + ); + }) + } + + #[test] + fn all_in_one_signed_submission_scenario() { + // a combination of: + // - good_solution_is_rewarded + // - bad_solution_is_slashed + // - suppressed_solution_gets_bond_back + ExtBuilder::default().build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + assert_eq!(balances(&99), (100, 0)); + assert_eq!(balances(&999), (100, 0)); + assert_eq!(balances(&9999), (100, 0)); + let solution = raw_solution(); + + // submit a correct one. + assert_ok!(submit_with_witness(Origin::signed(99), solution.clone())); + + // make the solution invalidly better and submit. This ought to be slashed. + let mut solution_999 = solution.clone(); + solution_999.score[0] += 1; + assert_ok!(submit_with_witness(Origin::signed(999), solution_999)); + + // make the solution invalidly worse and submit. This ought to be suppressed and + // returned. + let mut solution_9999 = solution.clone(); + solution_9999.score[0] -= 1; + assert_ok!(submit_with_witness(Origin::signed(9999), solution_9999)); + + assert_eq!( + MultiPhase::signed_submissions().iter().map(|x| x.who).collect::>(), + vec![9999, 99, 999] + ); + + // _some_ good solution was stored. + assert!(MultiPhase::finalize_signed_phase().0); + + // 99 is rewarded. + assert_eq!(balances(&99), (100 + 7, 0)); + // 999 is slashed. + assert_eq!(balances(&999), (95, 0)); + // 9999 gets everything back. + assert_eq!(balances(&9999), (100, 0)); + }) + } + + #[test] + fn cannot_consume_too_much_future_weight() { + ExtBuilder::default().signed_weight(40).mock_weight_info(true).build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + let (solution, witness) = MultiPhase::mine_solution(2).unwrap(); + let solution_weight = ::WeightInfo::feasibility_check( + witness.voters, + witness.targets, + solution.compact.voter_count() as u32, + solution.compact.unique_targets().len() as u32, + ); + // default solution will have 5 edges (5 * 5 + 10) + assert_eq!(solution_weight, 35); + assert_eq!(solution.compact.voter_count(), 5); + assert_eq!(::SignedMaxWeight::get(), 40); + + assert_ok!(submit_with_witness(Origin::signed(99), solution.clone())); + + ::set(30); + + // note: resubmitting the same solution is technically okay as long as the queue has + // space. + assert_noop!( + submit_with_witness(Origin::signed(99), solution), + Error::::SignedTooMuchWeight, + ); + }) + } + + #[test] + fn insufficient_deposit_doesnt_store_submission() { + ExtBuilder::default().build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + let solution = raw_solution(); + + assert_eq!(balances(&123), (0, 0)); + assert_noop!( + submit_with_witness(Origin::signed(123), solution), + Error::::SignedCannotPayDeposit, + ); + + assert_eq!(balances(&123), (0, 0)); + }) + } + + // given a full queue, and a solution which _should_ be allowed in, but the proposer of this + // new solution has insufficient deposit, we should not modify storage at all + #[test] + fn insufficient_deposit_with_full_queue_works_properly() { + ExtBuilder::default().build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + for s in 0..SignedMaxSubmissions::get() { + // score is always getting better + let solution = RawSolution { score: [(5 + s).into(), 0, 0], ..Default::default() }; + assert_ok!(submit_with_witness(Origin::signed(99), solution)); + } + + // this solution has a higher score than any in the queue + let solution = RawSolution { + score: [(5 + SignedMaxSubmissions::get()).into(), 0, 0], + ..Default::default() + }; + + assert_eq!(balances(&123), (0, 0)); + assert_noop!( + submit_with_witness(Origin::signed(123), solution), + Error::::SignedCannotPayDeposit, + ); + + assert_eq!(balances(&123), (0, 0)); + }) + } + + #[test] + fn finalize_signed_phase_is_idempotent_given_no_submissions() { + ExtBuilder::default().build_and_execute(|| { + for block_number in 0..25 { + roll_to(block_number); + + assert_eq!(SignedSubmissions::::decode_len().unwrap_or_default(), 0); + assert_storage_noop!(MultiPhase::finalize_signed_phase()); + } + }) + } + + #[test] + fn finalize_signed_phase_is_idempotent_given_submissions() { + ExtBuilder::default().build_and_execute(|| { + roll_to(15); + assert!(MultiPhase::current_phase().is_signed()); + + let solution = raw_solution(); + + // submit a correct one. + assert_ok!(submit_with_witness(Origin::signed(99), solution.clone())); + + // _some_ good solution was stored. + assert!(MultiPhase::finalize_signed_phase().0); + + // calling it again doesn't change anything + assert_storage_noop!(MultiPhase::finalize_signed_phase()); + }) + } +} diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index 543883fc035c5..52ecae7afa5fc 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -30,8 +30,10 @@ use sp_npos_elections::{ assignment_staked_to_ratio_normalized, is_score_better, seq_phragmen, }; use sp_runtime::{ + DispatchError, + SaturatedConversion, offchain::storage::{MutateStorageError, StorageValueRef}, - traits::TrailingZeroInput, SaturatedConversion + traits::TrailingZeroInput, }; use sp_std::{cmp::Ordering, convert::TryFrom, vec::Vec}; @@ -57,7 +59,8 @@ pub type Assignment = sp_npos_elections::Assignment< CompactAccuracyOf, >; -/// The [`IndexAssignment`][sp_npos_elections::IndexAssignment] type specialized for a particular runtime `T`. +/// The [`IndexAssignment`][sp_npos_elections::IndexAssignment] type specialized for a particular +/// runtime `T`. pub type IndexAssignmentOf = sp_npos_elections::IndexAssignmentOf>; #[derive(Debug, Eq, PartialEq)] @@ -69,7 +72,7 @@ pub enum MinerError { /// Submitting a transaction to the pool failed. PoolSubmissionFailed, /// The pre-dispatch checks failed for the mined solution. - PreDispatchChecksFailed, + PreDispatchChecksFailed(DispatchError), /// The solution generated from the miner is not feasible. Feasibility(FeasibilityError), /// Something went wrong fetching the lock. @@ -234,7 +237,7 @@ impl Pallet { ) -> Result<(), MinerError> { Self::unsigned_pre_dispatch_checks(raw_solution).map_err(|err| { log!(debug, "pre-dispatch checks failed for {} solution: {:?}", solution_type, err); - MinerError::PreDispatchChecksFailed + MinerError::PreDispatchChecksFailed(err) })?; Self::feasibility_check(raw_solution.clone(), ElectionCompute::Unsigned).map_err(|err| { @@ -344,7 +347,11 @@ impl Pallet { // converting to `Compact`. let mut index_assignments = sorted_assignments .into_iter() - .map(|assignment| IndexAssignmentOf::::new(&assignment, &voter_index, &target_index)) + .map(|assignment| IndexAssignmentOf::::new( + &assignment, + &voter_index, + &target_index, + )) .collect::, _>>()?; // trim assignments list for weight and length. @@ -416,7 +423,9 @@ impl Pallet { size, max_weight, ); - let removing: usize = assignments.len().saturating_sub(maximum_allowed_voters.saturated_into()); + let removing: usize = assignments.len().saturating_sub( + maximum_allowed_voters.saturated_into(), + ); log!( debug, "from {} assignments, truncating to {} for weight, removing {}", @@ -464,7 +473,9 @@ impl Pallet { } } let maximum_allowed_voters = - if low < assignments.len() && encoded_size_of(&assignments[..low + 1])? <= max_allowed_length { + if low < assignments.len() && + encoded_size_of(&assignments[..low + 1])? <= max_allowed_length + { low + 1 } else { low @@ -674,6 +685,15 @@ mod max_weight { fn on_initialize_open_unsigned_without_snapshot() -> Weight { unreachable!() } + fn finalize_signed_phase_accept_solution() -> Weight { + unreachable!() + } + fn finalize_signed_phase_reject_solution() -> Weight { + unreachable!() + } + fn submit(c: u32) -> Weight { + unreachable!() + } fn submit_unsigned(v: u32, t: u32, a: u32, d: u32) -> Weight { (0 * v + 0 * t + 1000 * a + 0 * d) as Weight } @@ -994,7 +1014,11 @@ mod tests { assert_eq!( MultiPhase::mine_check_save_submit().unwrap_err(), - MinerError::PreDispatchChecksFailed, + MinerError::PreDispatchChecksFailed(DispatchError::Module{ + index: 2, + error: 1, + message: Some("PreDispatchWrongWinnerCount"), + }), ); }) } @@ -1199,11 +1223,17 @@ mod tests { let mut storage = StorageValueRef::persistent(&OFFCHAIN_LAST_BLOCK); storage.clear(); - assert!(!ocw_solution_exists::(), "no solution should be present before we mine one"); + assert!( + !ocw_solution_exists::(), + "no solution should be present before we mine one", + ); // creates and cache a solution MultiPhase::offchain_worker(25); - assert!(ocw_solution_exists::(), "a solution must be cached after running the worker"); + assert!( + ocw_solution_exists::(), + "a solution must be cached after running the worker", + ); // after an election, the solution must be cleared // we don't actually care about the result of the election @@ -1329,10 +1359,15 @@ mod tests { _ => panic!("bad call: unexpected submission"), }; - // Custom(3) maps to PreDispatchChecksFailed - let pre_dispatch_check_error = TransactionValidityError::Invalid(InvalidTransaction::Custom(3)); + // Custom(7) maps to PreDispatchChecksFailed + let pre_dispatch_check_error = TransactionValidityError::Invalid( + InvalidTransaction::Custom(7), + ); assert_eq!( - ::validate_unsigned(TransactionSource::Local, &call) + ::validate_unsigned( + TransactionSource::Local, + &call, + ) .unwrap_err(), pre_dispatch_check_error, ); @@ -1359,7 +1394,11 @@ mod tests { let compact_clone = compact.clone(); // when - MultiPhase::trim_assignments_length(encoded_len, &mut assignments, encoded_size_of).unwrap(); + MultiPhase::trim_assignments_length( + encoded_len, + &mut assignments, + encoded_size_of, + ).unwrap(); // then let compact = CompactOf::::try_from(assignments.as_slice()).unwrap(); @@ -1383,7 +1422,11 @@ mod tests { let compact_clone = compact.clone(); // when - MultiPhase::trim_assignments_length(encoded_len as u32 - 1, &mut assignments, encoded_size_of).unwrap(); + MultiPhase::trim_assignments_length( + encoded_len as u32 - 1, + &mut assignments, + encoded_size_of, + ).unwrap(); // then let compact = CompactOf::::try_from(assignments.as_slice()).unwrap(); @@ -1414,7 +1457,11 @@ mod tests { .unwrap(); // when - MultiPhase::trim_assignments_length(encoded_len - 1, &mut assignments, encoded_size_of).unwrap(); + MultiPhase::trim_assignments_length( + encoded_len - 1, + &mut assignments, + encoded_size_of, + ).unwrap(); // then assert_eq!(assignments.len(), count - 1, "we must have removed exactly one assignment"); diff --git a/frame/election-provider-multi-phase/src/weights.rs b/frame/election-provider-multi-phase/src/weights.rs index 51b99bc962d43..6a245ebb51254 100644 --- a/frame/election-provider-multi-phase/src/weights.rs +++ b/frame/election-provider-multi-phase/src/weights.rs @@ -18,7 +18,7 @@ //! Autogenerated weights for pallet_election_provider_multi_phase //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0 -//! DATE: 2021-06-19, STEPS: `[50, ]`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2021-06-20, STEPS: `[50, ]`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128 // Executed Command: @@ -47,8 +47,11 @@ pub trait WeightInfo { fn on_initialize_nothing() -> Weight; fn on_initialize_open_signed() -> Weight; fn on_initialize_open_unsigned_with_snapshot() -> Weight; + fn finalize_signed_phase_accept_solution() -> Weight; + fn finalize_signed_phase_reject_solution() -> Weight; fn on_initialize_open_unsigned_without_snapshot() -> Weight; fn elect_queued() -> Weight; + fn submit(c: u32, ) -> Weight; fn submit_unsigned(v: u32, t: u32, a: u32, d: u32, ) -> Weight; fn feasibility_check(v: u32, t: u32, a: u32, d: u32, ) -> Weight; } @@ -57,52 +60,69 @@ pub trait WeightInfo { pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { fn on_initialize_nothing() -> Weight { - (24_579_000 as Weight) + (33_392_000 as Weight) .saturating_add(T::DbWeight::get().reads(8 as Weight)) } fn on_initialize_open_signed() -> Weight { - (87_463_000 as Weight) + (115_659_000 as Weight) .saturating_add(T::DbWeight::get().reads(10 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } fn on_initialize_open_unsigned_with_snapshot() -> Weight { - (87_381_000 as Weight) + (114_970_000 as Weight) .saturating_add(T::DbWeight::get().reads(10 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } + fn finalize_signed_phase_accept_solution() -> Weight { + (51_442_000 as Weight) + .saturating_add(T::DbWeight::get().reads(1 as Weight)) + .saturating_add(T::DbWeight::get().writes(2 as Weight)) + } + fn finalize_signed_phase_reject_solution() -> Weight { + (23_160_000 as Weight) + .saturating_add(T::DbWeight::get().reads(1 as Weight)) + .saturating_add(T::DbWeight::get().writes(1 as Weight)) + } fn on_initialize_open_unsigned_without_snapshot() -> Weight { - (18_489_000 as Weight) + (24_101_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn elect_queued() -> Weight { - (6_038_989_000 as Weight) - .saturating_add(T::DbWeight::get().reads(2 as Weight)) - .saturating_add(T::DbWeight::get().writes(6 as Weight)) + (6_153_604_000 as Weight) + .saturating_add(T::DbWeight::get().reads(5 as Weight)) + .saturating_add(T::DbWeight::get().writes(8 as Weight)) + } + fn submit(c: u32, ) -> Weight { + (78_972_000 as Weight) + // Standard Error: 16_000 + .saturating_add((308_000 as Weight).saturating_mul(c as Weight)) + .saturating_add(T::DbWeight::get().reads(4 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn submit_unsigned(v: u32, t: u32, a: u32, d: u32, ) -> Weight { (0 as Weight) // Standard Error: 12_000 - .saturating_add((3_480_000 as Weight).saturating_mul(v as Weight)) + .saturating_add((3_572_000 as Weight).saturating_mul(v as Weight)) // Standard Error: 42_000 - .saturating_add((194_000 as Weight).saturating_mul(t as Weight)) + .saturating_add((23_000 as Weight).saturating_mul(t as Weight)) // Standard Error: 12_000 - .saturating_add((10_498_000 as Weight).saturating_mul(a as Weight)) + .saturating_add((11_529_000 as Weight).saturating_mul(a as Weight)) // Standard Error: 63_000 - .saturating_add((3_074_000 as Weight).saturating_mul(d as Weight)) + .saturating_add((3_333_000 as Weight).saturating_mul(d as Weight)) .saturating_add(T::DbWeight::get().reads(7 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn feasibility_check(v: u32, t: u32, a: u32, d: u32, ) -> Weight { (0 as Weight) // Standard Error: 7_000 - .saturating_add((3_481_000 as Weight).saturating_mul(v as Weight)) - // Standard Error: 24_000 - .saturating_add((385_000 as Weight).saturating_mul(t as Weight)) + .saturating_add((3_647_000 as Weight).saturating_mul(v as Weight)) + // Standard Error: 23_000 + .saturating_add((390_000 as Weight).saturating_mul(t as Weight)) // Standard Error: 7_000 - .saturating_add((8_538_000 as Weight).saturating_mul(a as Weight)) - // Standard Error: 36_000 - .saturating_add((3_322_000 as Weight).saturating_mul(d as Weight)) + .saturating_add((9_614_000 as Weight).saturating_mul(a as Weight)) + // Standard Error: 35_000 + .saturating_add((3_405_000 as Weight).saturating_mul(d as Weight)) .saturating_add(T::DbWeight::get().reads(4 as Weight)) } } @@ -110,52 +130,69 @@ impl WeightInfo for SubstrateWeight { // For backwards compatibility and tests impl WeightInfo for () { fn on_initialize_nothing() -> Weight { - (24_579_000 as Weight) + (33_392_000 as Weight) .saturating_add(RocksDbWeight::get().reads(8 as Weight)) } fn on_initialize_open_signed() -> Weight { - (87_463_000 as Weight) + (115_659_000 as Weight) .saturating_add(RocksDbWeight::get().reads(10 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } fn on_initialize_open_unsigned_with_snapshot() -> Weight { - (87_381_000 as Weight) + (114_970_000 as Weight) .saturating_add(RocksDbWeight::get().reads(10 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } + fn finalize_signed_phase_accept_solution() -> Weight { + (51_442_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(1 as Weight)) + .saturating_add(RocksDbWeight::get().writes(2 as Weight)) + } + fn finalize_signed_phase_reject_solution() -> Weight { + (23_160_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(1 as Weight)) + .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + } fn on_initialize_open_unsigned_without_snapshot() -> Weight { - (18_489_000 as Weight) + (24_101_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn elect_queued() -> Weight { - (6_038_989_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(2 as Weight)) - .saturating_add(RocksDbWeight::get().writes(6 as Weight)) + (6_153_604_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) + .saturating_add(RocksDbWeight::get().writes(8 as Weight)) + } + fn submit(c: u32, ) -> Weight { + (78_972_000 as Weight) + // Standard Error: 16_000 + .saturating_add((308_000 as Weight).saturating_mul(c as Weight)) + .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn submit_unsigned(v: u32, t: u32, a: u32, d: u32, ) -> Weight { (0 as Weight) // Standard Error: 12_000 - .saturating_add((3_480_000 as Weight).saturating_mul(v as Weight)) + .saturating_add((3_572_000 as Weight).saturating_mul(v as Weight)) // Standard Error: 42_000 - .saturating_add((194_000 as Weight).saturating_mul(t as Weight)) + .saturating_add((23_000 as Weight).saturating_mul(t as Weight)) // Standard Error: 12_000 - .saturating_add((10_498_000 as Weight).saturating_mul(a as Weight)) + .saturating_add((11_529_000 as Weight).saturating_mul(a as Weight)) // Standard Error: 63_000 - .saturating_add((3_074_000 as Weight).saturating_mul(d as Weight)) + .saturating_add((3_333_000 as Weight).saturating_mul(d as Weight)) .saturating_add(RocksDbWeight::get().reads(7 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn feasibility_check(v: u32, t: u32, a: u32, d: u32, ) -> Weight { (0 as Weight) // Standard Error: 7_000 - .saturating_add((3_481_000 as Weight).saturating_mul(v as Weight)) - // Standard Error: 24_000 - .saturating_add((385_000 as Weight).saturating_mul(t as Weight)) + .saturating_add((3_647_000 as Weight).saturating_mul(v as Weight)) + // Standard Error: 23_000 + .saturating_add((390_000 as Weight).saturating_mul(t as Weight)) // Standard Error: 7_000 - .saturating_add((8_538_000 as Weight).saturating_mul(a as Weight)) - // Standard Error: 36_000 - .saturating_add((3_322_000 as Weight).saturating_mul(d as Weight)) + .saturating_add((9_614_000 as Weight).saturating_mul(a as Weight)) + // Standard Error: 35_000 + .saturating_add((3_405_000 as Weight).saturating_mul(d as Weight)) .saturating_add(RocksDbWeight::get().reads(4 as Weight)) } } diff --git a/frame/support/src/storage/bounded_btree_map.rs b/frame/support/src/storage/bounded_btree_map.rs index 8c50557618eec..0c1994d63a35d 100644 --- a/frame/support/src/storage/bounded_btree_map.rs +++ b/frame/support/src/storage/bounded_btree_map.rs @@ -39,7 +39,8 @@ pub struct BoundedBTreeMap(BTreeMap, PhantomData); impl Decode for BoundedBTreeMap where - BTreeMap: Decode, + K: Decode + Ord, + V: Decode, S: Get, { fn decode(input: &mut I) -> Result { @@ -115,14 +116,15 @@ where self.0.get_mut(key) } - /// Exactly the same semantics as [`BTreeMap::insert`], but returns an `Err` (and is a noop) if the - /// new length of the map exceeds `S`. - pub fn try_insert(&mut self, key: K, value: V) -> Result<(), ()> { - if self.len() < Self::bound() { - self.0.insert(key, value); - Ok(()) + /// Exactly the same semantics as [`BTreeMap::insert`], but returns an `Err` (and is a noop) if + /// the new length of the map exceeds `S`. + /// + /// In the `Err` case, returns the inserted pair so it can be further used without cloning. + pub fn try_insert(&mut self, key: K, value: V) -> Result, (K, V)> { + if self.len() < Self::bound() || self.0.contains_key(&key) { + Ok(self.0.insert(key, value)) } else { - Err(()) + Err((key, value)) } } @@ -407,4 +409,50 @@ pub mod test { Err("BoundedBTreeMap exceeds its limit".into()), ); } + + #[test] + fn unequal_eq_impl_insert_works() { + // given a struct with a strange notion of equality + #[derive(Debug)] + struct Unequal(u32, bool); + + impl PartialEq for Unequal { + fn eq(&self, other: &Self) -> bool { + self.0 == other.0 + } + } + impl Eq for Unequal {} + + impl Ord for Unequal { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.0.cmp(&other.0) + } + } + + impl PartialOrd for Unequal { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } + } + + let mut map = BoundedBTreeMap::::new(); + + // when the set is full + + for i in 0..4 { + map.try_insert(Unequal(i, false), i).unwrap(); + } + + // can't insert a new distinct member + map.try_insert(Unequal(5, false), 5).unwrap_err(); + + // but _can_ insert a distinct member which compares equal, though per the documentation, + // neither the set length nor the actual member are changed, but the value is + map.try_insert(Unequal(0, true), 6).unwrap(); + assert_eq!(map.len(), 4); + let (zero_key, zero_value) = map.get_key_value(&Unequal(0, true)).unwrap(); + assert_eq!(zero_key.0, 0); + assert_eq!(zero_key.1, false); + assert_eq!(*zero_value, 6); + } } diff --git a/frame/support/src/storage/bounded_btree_set.rs b/frame/support/src/storage/bounded_btree_set.rs index f551a3cbfa38e..10c2300a08a09 100644 --- a/frame/support/src/storage/bounded_btree_set.rs +++ b/frame/support/src/storage/bounded_btree_set.rs @@ -39,7 +39,7 @@ pub struct BoundedBTreeSet(BTreeSet, PhantomData); impl Decode for BoundedBTreeSet where - BTreeSet: Decode, + T: Decode + Ord, S: Get, { fn decode(input: &mut I) -> Result { @@ -103,14 +103,15 @@ where self.0.clear() } - /// Exactly the same semantics as [`BTreeSet::insert`], but returns an `Err` (and is a noop) if the - /// new length of the set exceeds `S`. - pub fn try_insert(&mut self, item: T) -> Result<(), ()> { - if self.len() < Self::bound() { - self.0.insert(item); - Ok(()) + /// Exactly the same semantics as [`BTreeSet::insert`], but returns an `Err` (and is a noop) if + /// the new length of the set exceeds `S`. + /// + /// In the `Err` case, returns the inserted item so it can be further used without cloning. + pub fn try_insert(&mut self, item: T) -> Result { + if self.len() < Self::bound() || self.0.contains(&item) { + Ok(self.0.insert(item)) } else { - Err(()) + Err(item) } } @@ -393,4 +394,49 @@ pub mod test { Err("BoundedBTreeSet exceeds its limit".into()), ); } + + #[test] + fn unequal_eq_impl_insert_works() { + // given a struct with a strange notion of equality + #[derive(Debug)] + struct Unequal(u32, bool); + + impl PartialEq for Unequal { + fn eq(&self, other: &Self) -> bool { + self.0 == other.0 + } + } + impl Eq for Unequal {} + + impl Ord for Unequal { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.0.cmp(&other.0) + } + } + + impl PartialOrd for Unequal { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } + } + + let mut set = BoundedBTreeSet::::new(); + + // when the set is full + + for i in 0..4 { + set.try_insert(Unequal(i, false)).unwrap(); + } + + // can't insert a new distinct member + set.try_insert(Unequal(5, false)).unwrap_err(); + + // but _can_ insert a distinct member which compares equal, though per the documentation, + // neither the set length nor the actual member are changed + set.try_insert(Unequal(0, true)).unwrap(); + assert_eq!(set.len(), 4); + let zero_item = set.get(&Unequal(0, true)).unwrap(); + assert_eq!(zero_item.0, 0); + assert_eq!(zero_item.1, false); + } } diff --git a/primitives/npos-elections/compact/src/lib.rs b/primitives/npos-elections/compact/src/lib.rs index e8cde87744539..0e9fbb34eea17 100644 --- a/primitives/npos-elections/compact/src/lib.rs +++ b/primitives/npos-elections/compact/src/lib.rs @@ -169,7 +169,7 @@ fn struct_def( ); quote!{ #compact_impl - #[derive(Default, PartialEq, Eq, Clone, Debug)] + #[derive(Default, PartialEq, Eq, Clone, Debug, PartialOrd, Ord)] } } else { // automatically derived.