From be93172f10ad3f6af7308e1243475b999d50107d Mon Sep 17 00:00:00 2001 From: Konrad Stepniak Date: Mon, 1 Jul 2024 16:22:30 +0200 Subject: [PATCH 01/16] test(pallet-market): add code coverage tools --- .gitignore | 7 ++++++- .vscode/settings.json | 7 +++++++ DEVELOPMENT.md | 5 +++++ Justfile | 11 +++++++++++ flake.lock | 12 ++++++------ flake.nix | 1 + rust-toolchain.toml | 2 +- 7 files changed, 37 insertions(+), 8 deletions(-) create mode 100644 .vscode/settings.json diff --git a/.gitignore b/.gitignore index d541f6c74..9c0790d96 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,9 @@ .AppleDouble .LSOverride .idea -.vscode +.vscode/* +# For team-wide VS Code configuration, like code-coverage extensions and previews +!.vscode/settings.json # Rust .gitignore # https://github.com/github/gitignore/blob/main/Rust.gitignore @@ -21,3 +23,6 @@ target/ # reproducible local environment .direnv + +# code coverage +coverage/ \ No newline at end of file diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 000000000..7458a6a98 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,7 @@ +{ + "coverage-gutters.coverageBaseDir": "coverage", + "coverage-gutters.coverageFileNames": [ + "pallet-market.lcov.info", + "mater.lcov.info" + ] +} diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 78744027d..ad98b74c4 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -10,6 +10,11 @@ - [direnv](https://direnv.net/) with a [shell hook](https://direnv.net/docs/hook.html) - *VS Code only* [direnv extension](https://marketplace.visualstudio.com/items?itemName=mkhl.direnv) (uses the same tooling as rust-toolchain.toml defined). - reasoning: when you enter a directory it uses everything defined in [.envrc](./.envrc), e.g. environment variables, `nix`, secrets. +- [cargo-llvm-cov](llvm coverage generator), produces coverage in `coverage/` directory. + - *VS Code only* [Coverage Gutters extension](https://github.com/ryanluker/vscode-coverage-gutters), configured in `.vscode/settings.json`. + +> [!NOTE] +> `.vscode/settings.json` is part of the git repo, please add only necessary and life-saving things there (like Code Coverage configuration). ## How it works? Nix is a package manager, which sneakily downloads all of the dependencies and updates PATH when you launch it with `nix develop`. diff --git a/Justfile b/Justfile index 35241e0c6..92746e099 100644 --- a/Justfile +++ b/Justfile @@ -15,3 +15,14 @@ release: lint testnet: release zombienet -p native spawn zombienet/local-testnet.toml +# Must be in sync with .vscode/settings.json and have extension Coverage Gutters to display it in VS Code. +market-coverage: + mkdir -p coverage + cargo llvm-cov -p pallet-market --ignore-filename-regex "(mock|test)" + cargo llvm-cov -p pallet-market report --ignore-filename-regex "(mock|test)" --html --output-dir coverage/pallet-market + cargo llvm-cov -p pallet-market report --ignore-filename-regex "(mock|test)" --lcov --output-path coverage/pallet-market.lcov.info + +mater-coverage: + cargo llvm-cov -p mater --ignore-filename-regex "(mock|test)" + cargo llvm-cov -p mater report --ignore-filename-regex "(mock|test)" --html --output-dir coverage/mater + cargo llvm-cov -p mater report --ignore-filename-regex "(mock|test)" --lcov --output-path coverage/mater.lcov.info \ No newline at end of file diff --git a/flake.lock b/flake.lock index 2caf7b626..76413096c 100644 --- a/flake.lock +++ b/flake.lock @@ -37,11 +37,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1718714799, - "narHash": "sha256-FUZpz9rg3gL8NVPKbqU8ei1VkPLsTIfAJ2fdAf5qjak=", + "lastModified": 1719690277, + "narHash": "sha256-0xSej1g7eP2kaUF+JQp8jdyNmpmCJKRpO12mKl/36Kc=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "c00d587b1a1afbf200b1d8f0b0e4ba9deb1c7f0e", + "rev": "2741b4b489b55df32afac57bc4bfd220e8bf617e", "type": "github" }, "original": { @@ -99,11 +99,11 @@ ] }, "locked": { - "lastModified": 1718936281, - "narHash": "sha256-jslEDCVFoRcNilJT0xYGSxqMjOe+USnLknpHIAZJ02A=", + "lastModified": 1719800573, + "narHash": "sha256-9DLgG4T6l7cc4pJNOCcXGUwHsFfUp8KLsiwed65MdHk=", "owner": "oxalica", "repo": "rust-overlay", - "rev": "c9a793a5278f711a59fe77b9bf54b215667022c6", + "rev": "648b25dd9c3acd255dc50c1eb3ca8b987856f675", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index 820208655..1a46ef785 100644 --- a/flake.nix +++ b/flake.nix @@ -23,6 +23,7 @@ }; rustToolchain = pkgs.pkgsBuildHost.rust-bin.fromRustupToolchainFile ./rust-toolchain.toml; buildInputs = with pkgs; [ + cargo-llvm-cov clang pkg-config rustToolchain diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 47df6fabc..9ed71b738 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,5 +1,5 @@ [toolchain] channel = "1.77.0" -components = ["cargo", "clippy", "rust-analyzer", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] +components = ["cargo", "clippy", "rust-analyzer", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt", "llvm-tools-preview"] profile = "minimal" targets = ["wasm32-unknown-unknown"] From d39c00b2a22aadb55466669030c64949a40fa6ea Mon Sep 17 00:00:00 2001 From: Konrad Stepniak Date: Mon, 1 Jul 2024 18:40:43 +0200 Subject: [PATCH 02/16] docs: add recommended vs code extensions to make developers' life easier --- .gitignore | 4 +++- .vscode/extensions.json | 16 ++++++++++++++++ DEVELOPMENT.md | 5 ++--- 3 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 .vscode/extensions.json diff --git a/.gitignore b/.gitignore index 9c0790d96..51b65501c 100644 --- a/.gitignore +++ b/.gitignore @@ -5,7 +5,9 @@ .idea .vscode/* # For team-wide VS Code configuration, like code-coverage extensions and previews -!.vscode/settings.json +!.vscode/settings.json +# Recommended extensions for VS Code +!.vscode/extensions.json # Rust .gitignore # https://github.com/github/gitignore/blob/main/Rust.gitignore diff --git a/.vscode/extensions.json b/.vscode/extensions.json new file mode 100644 index 000000000..20339f51d --- /dev/null +++ b/.vscode/extensions.json @@ -0,0 +1,16 @@ +{ + "recommendations": [ + // Rust language support + "1yib.rust-bundle", + // Justfile + "nefrob.vscode-just-syntax", + // TOML support + "tamasfe.even-better-toml", + // Code Coverage display inline, next to the code + "ryanluker.vscode-coverage-gutters", + // direnv, to use tool binaries from `nix` profile and set environment variables automatically + "mkhl.direnv", + // Inline information about crates + "serayuzgur.crates" + ] +} \ No newline at end of file diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index ad98b74c4..19b1b4b04 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -6,15 +6,14 @@ - [nix](https://nixos.org/download/) with [flakes](https://nixos.wiki/wiki/flakes) enabled (`echo 'experimental-features = nix-command flakes' >> ~/.config/nix/nix.conf`) - reasoning: every developer has the same version of development tools (rust, protoc, zombienet), directed by [flake.nix](./flake.nix)`. - how it works? fasterthanli.me has [a whole series on it](https://fasterthanli.me/series/building-a-rust-service-with-nix/part-10). - - optional: [vscode extension for Nix](https://marketplace.visualstudio.com/items?itemName=jnoortheen.nix-ide) - [direnv](https://direnv.net/) with a [shell hook](https://direnv.net/docs/hook.html) - - *VS Code only* [direnv extension](https://marketplace.visualstudio.com/items?itemName=mkhl.direnv) (uses the same tooling as rust-toolchain.toml defined). - reasoning: when you enter a directory it uses everything defined in [.envrc](./.envrc), e.g. environment variables, `nix`, secrets. - [cargo-llvm-cov](llvm coverage generator), produces coverage in `coverage/` directory. - - *VS Code only* [Coverage Gutters extension](https://github.com/ryanluker/vscode-coverage-gutters), configured in `.vscode/settings.json`. > [!NOTE] > `.vscode/settings.json` is part of the git repo, please add only necessary and life-saving things there (like Code Coverage configuration). +> +> [Recommended extensions](https://code.visualstudio.com/docs/editor/extension-marketplace#_workspace-recommended-extensions) are also part of the repo in `.vscode/extensions.json`. ## How it works? Nix is a package manager, which sneakily downloads all of the dependencies and updates PATH when you launch it with `nix develop`. From 1d7c80c63b4fcb3d21ff0f15e926cd3dcb19e69d Mon Sep 17 00:00:00 2001 From: Konrad Stepniak Date: Tue, 2 Jul 2024 15:33:47 +0200 Subject: [PATCH 03/16] test(pallet-market): publish_storage_deals failure tests --- pallets/market/src/mock.rs | 5 +- pallets/market/src/test.rs | 419 ++++++++++++++++++++++++++++++++----- 2 files changed, 371 insertions(+), 53 deletions(-) diff --git a/pallets/market/src/mock.rs b/pallets/market/src/mock.rs index 5364f8be4..a0d7ec4fd 100644 --- a/pallets/market/src/mock.rs +++ b/pallets/market/src/mock.rs @@ -56,7 +56,7 @@ impl crate::Config for Test { type OffchainPublic = AccountPublic; type MaxDeals = ConstU32<32>; type BlocksPerDay = ConstU64<1>; - type MinDealDuration = ConstU64<1>; + type MinDealDuration = ConstU64<2>; type MaxDealDuration = ConstU64<30>; type MaxDealsPerBlock = ConstU32<32>; } @@ -84,7 +84,7 @@ pub fn cid_of(data: &str) -> cid::Cid { pub(crate) type DealProposalOf = DealProposal<::AccountId, BalanceOf, BlockNumberFor>; -type ClientDealProposalOf = ClientDealProposal< +pub(crate) type ClientDealProposalOf = ClientDealProposal< ::AccountId, BalanceOf, BlockNumberFor, @@ -107,6 +107,7 @@ pub const INITIAL_FUNDS: u64 = 100; /// Build genesis storage according to the mock runtime. pub fn new_test_ext() -> sp_io::TestExternalities { + let _ = env_logger::try_init(); let mut t = system::GenesisConfig::::default() .build_storage() .unwrap() diff --git a/pallets/market/src/test.rs b/pallets/market/src/test.rs index e5a89d034..5a8d7f9af 100644 --- a/pallets/market/src/test.rs +++ b/pallets/market/src/test.rs @@ -3,7 +3,8 @@ use core::str::FromStr; use cid::Cid; use frame_support::{ assert_err, assert_noop, assert_ok, - sp_runtime::{bounded_vec, ArithmeticError, DispatchError, TokenError}, + pallet_prelude::ConstU32, + sp_runtime::{bounded_vec, ArithmeticError, BoundedVec, DispatchError, TokenError}, BoundedBTreeSet, }; use primitives_proofs::{ @@ -185,7 +186,7 @@ fn fails_to_withdraw_balance() { } #[test] -fn publish_storage_deals_fails_with_empty_deals() { +fn publish_storage_deals_fails_empty_deals() { new_test_ext().execute_with(|| { assert_noop!( Market::publish_storage_deals(RuntimeOrigin::signed(account(PROVIDER)), bounded_vec![]), @@ -195,48 +196,270 @@ fn publish_storage_deals_fails_with_empty_deals() { } #[test] -fn publish_storage_deals() { - let _ = env_logger::try_init(); +fn publish_storage_deals_fails_caller_not_provider() { + new_test_ext().execute_with(|| { + assert_noop!( + Market::publish_storage_deals( + RuntimeOrigin::signed(account(ALICE)), + bounded_vec![DealProposalBuilder::default().signed(ALICE)] + ), + Error::::ProposalsNotPublishedByStorageProvider + ); + }); +} +#[test] +fn publish_storage_deals_fails_invalid_signature() { new_test_ext().execute_with(|| { - let alice_start_block = 100; - let alice_proposal = sign_proposal( - ALICE, - DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, + let mut deal = DealProposalBuilder::default().signed(ALICE); + // Change the message contents so the signature does not match + deal.proposal.piece_size = 1337; + + assert_noop!( + Market::publish_storage_deals( + RuntimeOrigin::signed(account(PROVIDER)), + bounded_vec![deal] + ), + Error::::AllProposalsInvalid + ); + }); +} + +#[test] +fn publish_storage_deals_fails_end_before_start() { + new_test_ext().execute_with(|| { + let proposal = DealProposalBuilder::default() + // Make start_block > end_block + .start_block(1337) + .signed(ALICE); + + assert_noop!( + Market::publish_storage_deals( + RuntimeOrigin::signed(account(PROVIDER)), + bounded_vec![proposal] + ), + Error::::AllProposalsInvalid + ); + }); +} + +#[test] +fn publish_storage_deals_fails_must_be_unpublished() { + new_test_ext().execute_with(|| { + let proposal = DealProposalBuilder::default() + .state(DealState::Published) + .signed(ALICE); + + assert_noop!( + Market::publish_storage_deals( + RuntimeOrigin::signed(account(PROVIDER)), + bounded_vec![proposal] + ), + Error::::AllProposalsInvalid + ); + }); +} + +#[test] +fn publish_storage_deals_fails_min_duration_out_of_bounds() { + new_test_ext().execute_with(|| { + let proposal = DealProposalBuilder::default() + .start_block(10) + // Make duration shorter than [`T::MinDealDuration`] + .end_block(11) + .signed(ALICE); + + assert_noop!( + Market::publish_storage_deals( + RuntimeOrigin::signed(account(PROVIDER)), + bounded_vec![proposal] + ), + Error::::AllProposalsInvalid + ); + }); +} + +#[test] +fn publish_storage_deals_fails_max_duration_out_of_bounds() { + new_test_ext().execute_with(|| { + let proposal = DealProposalBuilder::default() + // Make duration too many blocks. + .end_block(1000) + .signed(ALICE); + + assert_noop!( + Market::publish_storage_deals( + RuntimeOrigin::signed(account(PROVIDER)), + bounded_vec![proposal] + ), + Error::::AllProposalsInvalid + ); + }); +} + +#[test] +fn publish_storage_deals_fails_different_providers() { + new_test_ext().execute_with(|| { + // Let the first proposal pass, by adding enough balance for Alice + // Second proposal will be rejected, but first still published + let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 60); + let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 60); + System::reset_events(); + + assert_ok!(Market::publish_storage_deals( + RuntimeOrigin::signed(account(PROVIDER)), + bounded_vec![ + DealProposalBuilder::default().signed(ALICE), + // Proposal where second deal's provider is not a caller + DealProposalBuilder::default() + .client(BOB) + .provider(BOB) + .signed(BOB), + ] + )); + assert_eq!( + events(), + [RuntimeEvent::Market(Event::::DealPublished { + deal_id: 0, client: account(ALICE), provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: alice_start_block, - end_block: 110, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Published, - }, + })] ); - let bob_start_block = 130; - let bob_proposal = sign_proposal( - BOB, - DealProposal { - piece_cid: cid_of("polka-storage-data-bob") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 21, - client: account(BOB), + }); +} + +#[test] +fn publish_storage_deals_fails_client_not_enough_funds_for_second_deal() { + new_test_ext().execute_with(|| { + // Let the first proposal pass, by adding enough balance for Alice + // Second proposal will be rejected, but first still published + let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 60); + let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 60); + System::reset_events(); + + assert_ok!(Market::publish_storage_deals( + RuntimeOrigin::signed(account(PROVIDER)), + bounded_vec![ + DealProposalBuilder::default().signed(ALICE), + DealProposalBuilder::default().signed(ALICE), + ] + )); + assert_eq!( + events(), + [RuntimeEvent::Market(Event::::DealPublished { + deal_id: 0, + client: account(ALICE), provider: account(PROVIDER), - label: bounded_vec![0xa, 0xe, 0xe, 0xf], - start_block: bob_start_block, - end_block: 135, - storage_price_per_block: 10, - provider_collateral: 15, - state: DealState::Published, - }, + })] + ); + }); +} + +#[test] +fn publish_storage_deals_fails_provider_not_enough_funds_for_second_deal() { + new_test_ext().execute_with(|| { + // Let the first proposal pass, by adding enough balance for Provider + // Collateral is 25 for the default deal, so provider should have at least 50. + // Second proposal will be rejected, but first still published + let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 40); + let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 90); + let _ = Market::add_balance(RuntimeOrigin::signed(account(BOB)), 90); + System::reset_events(); + + assert_ok!(Market::publish_storage_deals( + RuntimeOrigin::signed(account(PROVIDER)), + bounded_vec![ + DealProposalBuilder::default().signed(ALICE), + DealProposalBuilder::default().client(BOB).signed(BOB), + ] + )); + assert_eq!( + events(), + [RuntimeEvent::Market(Event::::DealPublished { + deal_id: 0, + client: account(ALICE), + provider: account(PROVIDER), + })] + ); + }); +} + +#[test] +fn publish_storage_deals_fails_duplicate_deal_in_message() { + new_test_ext().execute_with(|| { + let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 90); + let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 90); + System::reset_events(); + + assert_ok!(Market::publish_storage_deals( + RuntimeOrigin::signed(account(PROVIDER)), + bounded_vec![ + DealProposalBuilder::default() + .storage_price_per_block(1) + .signed(ALICE), + DealProposalBuilder::default() + .storage_price_per_block(1) + .signed(ALICE), + ] + )); + assert_eq!( + events(), + [RuntimeEvent::Market(Event::::DealPublished { + deal_id: 0, + client: account(ALICE), + provider: account(PROVIDER), + })] ); + }); +} + +#[test] +fn publish_storage_deals_fails_duplicate_deal_in_state() { + new_test_ext().execute_with(|| { + let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 90); + let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 90); + System::reset_events(); + + assert_ok!(Market::publish_storage_deals( + RuntimeOrigin::signed(account(PROVIDER)), + bounded_vec![DealProposalBuilder::default() + .storage_price_per_block(1) + .signed(ALICE),] + )); + assert_eq!( + events(), + [RuntimeEvent::Market(Event::::DealPublished { + deal_id: 0, + client: account(ALICE), + provider: account(PROVIDER), + })] + ); + assert_noop!( + Market::publish_storage_deals( + RuntimeOrigin::signed(account(PROVIDER)), + bounded_vec![DealProposalBuilder::default() + .storage_price_per_block(1) + .signed(ALICE),] + ), + Error::::AllProposalsInvalid + ); + }); +} + +#[test] +fn publish_storage_deals() { + new_test_ext().execute_with(|| { + let alice_proposal = DealProposalBuilder::default().signed(ALICE); + // We're not expecting for it to go through, but the call should not fail. + let alice_second_proposal = DealProposalBuilder::default().piece_size(37).signed(ALICE); + let bob_proposal = DealProposalBuilder::default() + .client(BOB) + .start_block(130) + .end_block(135) + .storage_price_per_block(10) + .provider_collateral(15) + .signed(BOB); + let alice_hash = Market::hash_proposal(&alice_proposal.proposal); let bob_hash = Market::hash_proposal(&bob_proposal.proposal); @@ -247,7 +470,11 @@ fn publish_storage_deals() { assert_ok!(Market::publish_storage_deals( RuntimeOrigin::signed(account(PROVIDER)), - bounded_vec![alice_proposal, bob_proposal] + bounded_vec![ + alice_proposal.clone(), + alice_second_proposal.clone(), + bob_proposal.clone() + ] )); assert_eq!( BalanceTable::::get(account(ALICE)), @@ -288,14 +515,13 @@ fn publish_storage_deals() { ); assert!(PendingProposals::::get().contains(&alice_hash)); assert!(PendingProposals::::get().contains(&bob_hash)); - assert!(DealsForBlock::::get(&alice_start_block).contains(&0)); - assert!(DealsForBlock::::get(&bob_start_block).contains(&1)); + assert!(DealsForBlock::::get(&alice_proposal.proposal.start_block).contains(&0)); + assert!(DealsForBlock::::get(&bob_proposal.proposal.start_block).contains(&1)); }); } #[test] fn verify_deals_for_activation() { - let _ = env_logger::try_init(); new_test_ext().execute_with(|| { publish_for_activation( 1, @@ -346,7 +572,6 @@ fn verify_deals_for_activation() { #[test] fn activate_deals() { - let _ = env_logger::try_init(); new_test_ext().execute_with(|| { let alice_hash = publish_for_activation( 1, @@ -425,8 +650,6 @@ fn publish_for_activation(deal_id: DealId, deal: DealProposalOf) -> H256 { #[test] fn verifies_deals_on_block_finalization() { - let _ = env_logger::try_init(); - new_test_ext().execute_with(|| { let alice_start_block = 100; let alice_proposal = sign_proposal( @@ -547,7 +770,6 @@ fn verifies_deals_on_block_finalization() { #[test] fn settle_deal_payments_not_found() { - let _ = env_logger::try_init(); new_test_ext().execute_with(|| { assert_ok!(Market::settle_deal_payments( RuntimeOrigin::signed(account(ALICE)), @@ -566,7 +788,6 @@ fn settle_deal_payments_not_found() { #[test] fn settle_deal_payments_early() { - let _ = env_logger::try_init(); new_test_ext().execute_with(|| { let alice_proposal = sign_proposal( ALICE, @@ -613,7 +834,6 @@ fn settle_deal_payments_early() { #[test] fn settle_deal_payments_published() { - let _ = env_logger::try_init(); new_test_ext().execute_with(|| { let alice_proposal = sign_proposal( ALICE, @@ -681,7 +901,6 @@ fn settle_deal_payments_published() { #[test] fn settle_deal_payments_active_future_last_update() { - let _ = env_logger::try_init(); new_test_ext().execute_with(|| { let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 60); let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 75); @@ -728,7 +947,6 @@ fn settle_deal_payments_active_future_last_update() { #[test] fn settle_deal_payments_active_corruption() { - let _ = env_logger::try_init(); new_test_ext().execute_with(|| { let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 60); let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 75); @@ -770,7 +988,6 @@ fn settle_deal_payments_active_corruption() { #[test] fn settle_deal_payments_success() { - let _ = env_logger::try_init(); new_test_ext().execute_with(|| { let alice_proposal = sign_proposal( ALICE, @@ -895,7 +1112,6 @@ fn settle_deal_payments_success() { #[test] fn settle_deal_payments_success_finished() { - let _ = env_logger::try_init(); new_test_ext().execute_with(|| { let alice_proposal = sign_proposal( ALICE, @@ -996,3 +1212,104 @@ fn settle_deal_payments_success_finished() { assert_eq!(Proposals::::get(0), None); }); } + +// TODO(@th7nder,01/07/2024): +// - refactor tests to use `example_proposal` after some of the stuff is merged +// - dispatch error/from +/// Builder to simplify writing complex tests of [`DealProposal`] +struct DealProposalBuilder { + piece_cid: BoundedVec>, + piece_size: u64, + client: AccountIdOf, + provider: AccountIdOf, + label: BoundedVec>, + start_block: u64, + end_block: u64, + storage_price_per_block: u64, + provider_collateral: u64, + state: DealState, +} + +impl Default for DealProposalBuilder { + fn default() -> Self { + Self { + piece_cid: cid_of("polka-storage-data") + .to_bytes() + .try_into() + .expect("hash is always 32 bytes"), + piece_size: 18, + client: account(ALICE), + provider: account(PROVIDER), + label: bounded_vec![0xb, 0xe, 0xe, 0xf], + start_block: 100, + end_block: 110, + storage_price_per_block: 5, + provider_collateral: 25, + // TODO(@th7nder,01/07/2024): change this to Published + state: DealState::Published, + } + } +} + +impl DealProposalBuilder { + pub fn client(mut self, client: &'static str) -> Self { + self.client = account(client); + self + } + + pub fn provider(mut self, provider: &'static str) -> Self { + self.provider = account(provider); + self + } + + pub fn state(mut self, state: DealState) -> Self { + self.state = state; + self + } + + pub fn start_block(mut self, start_block: u64) -> Self { + self.start_block = start_block; + self + } + + pub fn end_block(mut self, end_block: u64) -> Self { + self.end_block = end_block; + self + } + + pub fn storage_price_per_block(mut self, price: u64) -> Self { + self.storage_price_per_block = price; + self + } + + pub fn provider_collateral(mut self, price: u64) -> Self { + self.provider_collateral = price; + self + } + + pub fn piece_size(mut self, piece_size: u64) -> Self { + self.piece_size = piece_size; + self + } + + pub fn build(self) -> DealProposalOf { + DealProposalOf:: { + piece_cid: self.piece_cid, + piece_size: self.piece_size, + client: self.client, + provider: self.provider, + label: self.label, + start_block: self.start_block, + end_block: self.end_block, + storage_price_per_block: self.storage_price_per_block, + provider_collateral: self.provider_collateral, + state: self.state, + } + } + + pub fn signed(self, by: &'static str) -> ClientDealProposalOf { + let built = self.build(); + let signed = sign_proposal(by, built); + signed + } +} From a191127a455ba773a767dc048c6dd8bb9663f4a6 Mon Sep 17 00:00:00 2001 From: Konrad Stepniak Date: Wed, 3 Jul 2024 07:21:41 +0200 Subject: [PATCH 04/16] test(pallet-market): refactor and simplify tests to use DealProposalBuilder --- pallets/market/src/test.rs | 321 +++++++++---------------------------- 1 file changed, 79 insertions(+), 242 deletions(-) diff --git a/pallets/market/src/test.rs b/pallets/market/src/test.rs index 5a8d7f9af..7f85a38a2 100644 --- a/pallets/market/src/test.rs +++ b/pallets/market/src/test.rs @@ -523,24 +523,7 @@ fn publish_storage_deals() { #[test] fn verify_deals_for_activation() { new_test_ext().execute_with(|| { - publish_for_activation( - 1, - DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: 100, - end_block: 110, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Published, - }, - ); + publish_for_activation(1, DealProposalBuilder::default().build()); let deals = bounded_vec![ SectorDeal { @@ -573,24 +556,7 @@ fn verify_deals_for_activation() { #[test] fn activate_deals() { new_test_ext().execute_with(|| { - let alice_hash = publish_for_activation( - 1, - DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: 100, - end_block: 110, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Published, - }, - ); + let alice_hash = publish_for_activation(1, DealProposalBuilder::default().build()); let deals = bounded_vec![ SectorDeal { @@ -652,43 +618,21 @@ fn publish_for_activation(deal_id: DealId, deal: DealProposalOf) -> H256 { fn verifies_deals_on_block_finalization() { new_test_ext().execute_with(|| { let alice_start_block = 100; - let alice_proposal = sign_proposal( - ALICE, - DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: alice_start_block, - end_block: 110, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Published, - }, - ); + let alice_proposal = DealProposalBuilder::default() + .start_block(alice_start_block) + .end_block(alice_start_block + 10) + .storage_price_per_block(5) + .provider_collateral(25) + .signed(ALICE); + let bob_start_block = 130; - let bob_proposal = sign_proposal( - BOB, - DealProposal { - piece_cid: cid_of("polka-storage-data-bob") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 21, - client: account(BOB), - provider: account(PROVIDER), - label: bounded_vec![0xa, 0xe, 0xe, 0xf], - start_block: bob_start_block, - end_block: 135, - storage_price_per_block: 10, - provider_collateral: 15, - state: DealState::Published, - }, - ); + let bob_proposal = DealProposalBuilder::default() + .client(BOB) + .start_block(bob_start_block) + .end_block(bob_start_block + 5) + .storage_price_per_block(10) + .provider_collateral(15) + .signed(BOB); let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 60); let _ = Market::add_balance(RuntimeOrigin::signed(account(BOB)), 70); @@ -709,8 +653,7 @@ fn verifies_deals_on_block_finalization() { ); System::reset_events(); - // Idea: Activate Alice's Deal, forget to do that for Bob's. - + // Scenario: Activate Alice's Deal, forget to do that for Bob's. // Alice's balance before the hook assert_eq!( BalanceTable::::get(account(ALICE)), @@ -789,24 +732,7 @@ fn settle_deal_payments_not_found() { #[test] fn settle_deal_payments_early() { new_test_ext().execute_with(|| { - let alice_proposal = sign_proposal( - ALICE, - DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: 100, - end_block: 110, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Published, - }, - ); + let alice_proposal = DealProposalBuilder::default().signed(ALICE); let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 60); let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 75); @@ -835,24 +761,10 @@ fn settle_deal_payments_early() { #[test] fn settle_deal_payments_published() { new_test_ext().execute_with(|| { - let alice_proposal = sign_proposal( - ALICE, - DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: 0, - end_block: 10, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Published, - }, - ); + let alice_proposal = DealProposalBuilder::default() + .start_block(0) + .end_block(10) + .signed(ALICE); let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 60); let _ = Market::add_balance(RuntimeOrigin::signed(account(BOB)), 70); @@ -907,26 +819,16 @@ fn settle_deal_payments_active_future_last_update() { Proposals::::insert( 0, - DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: 0, - end_block: 10, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Active(ActiveDealState { + DealProposalBuilder::default() + .start_block(0) + .end_block(10) + .state(DealState::Active(ActiveDealState { sector_number: 0, sector_start_block: 0, last_updated_block: Some(10), slash_block: None, - }), - }, + })) + .build(), ); System::reset_events(); @@ -953,26 +855,16 @@ fn settle_deal_payments_active_corruption() { Proposals::::insert( 0, - DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: 0, - end_block: 10, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Active(ActiveDealState { + DealProposalBuilder::default() + .start_block(0) + .end_block(10) + .state(DealState::Active(ActiveDealState { sector_number: 0, sector_start_block: 0, last_updated_block: Some(11), slash_block: None, - }), - }, + })) + .build(), ); run_to_block(12); System::reset_events(); @@ -989,24 +881,10 @@ fn settle_deal_payments_active_corruption() { #[test] fn settle_deal_payments_success() { new_test_ext().execute_with(|| { - let alice_proposal = sign_proposal( - ALICE, - DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: 0, - end_block: 10, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Published, - }, - ); + let alice_proposal = DealProposalBuilder::default() + .start_block(0) + .end_block(10) + .signed(ALICE); let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 60); let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 75); @@ -1029,28 +907,19 @@ fn settle_deal_payments_success() { assert_eq!( Proposals::::get(0), - Some(DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: 0, - end_block: 10, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Active(ActiveDealState { - sector_number: 0, - sector_start_block: 0, - last_updated_block: None, - slash_block: None, - }), - }) + Some( + DealProposalBuilder::default() + .start_block(0) + .end_block(10) + .state(DealState::Active(ActiveDealState { + sector_number: 0, + sector_start_block: 0, + last_updated_block: None, + slash_block: None, + })) + .build() + ) ); - System::reset_events(); run_to_block(5); @@ -1086,26 +955,18 @@ fn settle_deal_payments_success() { assert_eq!( Proposals::::get(0), - Some(DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: 0, - end_block: 10, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Active(ActiveDealState { - sector_number: 0, - sector_start_block: 0, - last_updated_block: Some(5), - slash_block: None, - }), - }) + Some( + DealProposalBuilder::default() + .start_block(0) + .end_block(10) + .state(DealState::Active(ActiveDealState { + sector_number: 0, + sector_start_block: 0, + last_updated_block: Some(5), + slash_block: None, + })) + .build() + ) ); }); } @@ -1113,24 +974,10 @@ fn settle_deal_payments_success() { #[test] fn settle_deal_payments_success_finished() { new_test_ext().execute_with(|| { - let alice_proposal = sign_proposal( - ALICE, - DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: 0, - end_block: 10, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Published, - }, - ); + let alice_proposal = DealProposalBuilder::default() + .start_block(0) + .end_block(10) + .signed(ALICE); let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 60); let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 75); @@ -1153,26 +1000,18 @@ fn settle_deal_payments_success_finished() { assert_eq!( Proposals::::get(0), - Some(DealProposal { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: 0, - end_block: 10, - storage_price_per_block: 5, - provider_collateral: 25, - state: DealState::Active(ActiveDealState { - sector_number: 0, - sector_start_block: 0, - last_updated_block: None, - slash_block: None, - }), - }) + Some( + DealProposalBuilder::default() + .start_block(0) + .end_block(10) + .state(DealState::Active(ActiveDealState { + sector_number: 0, + sector_start_block: 0, + last_updated_block: None, + slash_block: None, + })) + .build() + ) ); System::reset_events(); @@ -1213,10 +1052,8 @@ fn settle_deal_payments_success_finished() { }); } -// TODO(@th7nder,01/07/2024): -// - refactor tests to use `example_proposal` after some of the stuff is merged -// - dispatch error/from /// Builder to simplify writing complex tests of [`DealProposal`] +/// Use of Generics is strictly limited and concrete values for the Test config are used instead. struct DealProposalBuilder { piece_cid: BoundedVec>, piece_size: u64, From a0755525078e6756fcd865477b4c0ee5e64c2dea Mon Sep 17 00:00:00 2001 From: Konrad Stepniak Date: Wed, 3 Jul 2024 17:02:13 +0200 Subject: [PATCH 05/16] test(pallet-market): verify_deals_for_activation failures --- pallets/market/src/test.rs | 131 +++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) diff --git a/pallets/market/src/test.rs b/pallets/market/src/test.rs index 7f85a38a2..23d428215 100644 --- a/pallets/market/src/test.rs +++ b/pallets/market/src/test.rs @@ -9,6 +9,7 @@ use frame_support::{ }; use primitives_proofs::{ ActiveDeal, ActiveSector, DealId, Market as MarketTrait, RegisteredSealProof, SectorDeal, + MAX_DEALS_PER_SECTOR, }; use sp_core::H256; @@ -553,6 +554,136 @@ fn verify_deals_for_activation() { }); } +/// Builder with nice defaults for test purposes. +struct SectorDealBuilder { + sector_number: u64, + sector_expiry: u64, + sector_type: RegisteredSealProof, + deal_ids: BoundedVec>, +} + +impl SectorDealBuilder { + pub fn sector_expiry(mut self, sector_expiry: u64) -> Self { + self.sector_expiry = sector_expiry; + self + } + + pub fn build(self) -> SectorDeal { + SectorDeal:: { + sector_number: self.sector_number, + sector_expiry: self.sector_expiry, + sector_type: self.sector_type, + deal_ids: self.deal_ids, + } + } +} + +impl Default for SectorDealBuilder { + fn default() -> Self { + Self { + sector_number: 1, + sector_expiry: 120, + sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, + deal_ids: bounded_vec![1], + } + } +} + +#[test] +fn verify_deals_for_activation_fails_with_different_provider() { + new_test_ext().execute_with(|| { + publish_for_activation(1, DealProposalBuilder::default().provider(BOB).build()); + + let deals = bounded_vec![SectorDealBuilder::default().build()]; + + assert_noop!( + Market::verify_deals_for_activation(&account(PROVIDER), deals), + Error::::DealActivationError + ); + }); +} + +#[test] +fn verify_deals_for_activation_fails_with_invalid_deal_state() { + new_test_ext().execute_with(|| { + publish_for_activation( + 1, + DealProposalBuilder::default() + .state(DealState::Active(ActiveDealState { + sector_number: 0, + sector_start_block: 0, + last_updated_block: Some(10), + slash_block: None, + })) + .build(), + ); + + let deals = bounded_vec![SectorDealBuilder::default().build()]; + + assert_noop!( + Market::verify_deals_for_activation(&account(PROVIDER), deals), + Error::::DealActivationError + ); + }); +} + +#[test] +fn verify_deals_for_activation_fails_deal_not_in_pending() { + new_test_ext().execute_with(|| { + // do not use `publish_for_activation` as it puts deal in PendingProposals + Proposals::::insert(1, DealProposalBuilder::default().build()); + let deals = bounded_vec![SectorDealBuilder::default().build()]; + + assert_noop!( + Market::verify_deals_for_activation(&account(PROVIDER), deals), + Error::::DealActivationError + ); + }); +} + +#[test] +fn verify_deals_for_activation_fails_sector_activation_after_start_block() { + new_test_ext().execute_with(|| { + // current_block == sector_activation when calling `verify_deals_for_activation` + // wait a couple of blocks so deal cannot be activated, because it's too late. + run_to_block(2); + + publish_for_activation( + 1, + DealProposalBuilder::default() + .start_block(1) + .build(), + ); + + let deals = bounded_vec![SectorDealBuilder::default().build()]; + + assert_noop!( + Market::verify_deals_for_activation(&account(PROVIDER), deals), + Error::::DealActivationError + ); + }); +} + +#[test] +fn verify_deals_for_activation_fails_sector_expires_before_deal_ends() { + new_test_ext().execute_with(|| { + publish_for_activation( + 1, + DealProposalBuilder::default() + .start_block(10) + .end_block(15) + .build(), + ); + + let deals = bounded_vec![SectorDealBuilder::default().sector_expiry(11).build()]; + + assert_noop!( + Market::verify_deals_for_activation(&account(PROVIDER), deals), + Error::::DealActivationError + ); + }); +} + #[test] fn activate_deals() { new_test_ext().execute_with(|| { From 4eabf2f62d78c6cec44c6ad87b48d24dc4ae86fb Mon Sep 17 00:00:00 2001 From: Konrad Stepniak Date: Thu, 4 Jul 2024 06:57:01 +0200 Subject: [PATCH 06/16] test(pallet-market): test activate_deals even more --- pallets/market/src/lib.rs | 6 +- pallets/market/src/test.rs | 155 ++++++++++++++++++++++++++++++++----- 2 files changed, 140 insertions(+), 21 deletions(-) diff --git a/pallets/market/src/lib.rs b/pallets/market/src/lib.rs index c8a73bd95..cd8aed61e 100644 --- a/pallets/market/src/lib.rs +++ b/pallets/market/src/lib.rs @@ -1135,7 +1135,11 @@ pub mod pallet { let mut pending_proposals = PendingProposals::::get(); for sector in sector_deals { - let proposals = Self::proposals_for_deals(sector.deal_ids)?; + let Ok(proposals) = Self::proposals_for_deals(sector.deal_ids) else { + log::error!("failed to find deals for sector: {}", sector.sector_number,); + continue; + }; + let sector_size = sector.sector_type.sector_size(); if let Err(e) = Self::validate_deals_for_sector( &proposals, diff --git a/pallets/market/src/test.rs b/pallets/market/src/test.rs index cfa42f99e..0f79bc15c 100644 --- a/pallets/market/src/test.rs +++ b/pallets/market/src/test.rs @@ -5,7 +5,6 @@ use frame_support::{ assert_err, assert_noop, assert_ok, pallet_prelude::ConstU32, sp_runtime::{bounded_vec, ArithmeticError, BoundedVec, DispatchError, TokenError}, - BoundedBTreeSet, }; use primitives_proofs::{ ActiveDeal, ActiveSector, DealId, Market as MarketTrait, RegisteredSealProof, SectorDeal, @@ -248,7 +247,12 @@ fn publish_storage_deals_fails_end_before_start() { fn publish_storage_deals_fails_must_be_unpublished() { new_test_ext().execute_with(|| { let proposal = DealProposalBuilder::default() - .state(DealState::Published) + .state(DealState::Active(ActiveDealState { + sector_number: 0, + sector_start_block: 0, + last_updated_block: Some(10), + slash_block: None, + })) .signed(ALICE); assert_noop!( @@ -568,6 +572,19 @@ impl SectorDealBuilder { self } + pub fn sector_number(mut self, sector_number: u64) -> Self { + self.sector_number = sector_number; + self + } + + pub fn deal_ids( + mut self, + deal_ids: BoundedVec>, + ) -> Self { + self.deal_ids = deal_ids; + self + } + pub fn build(self) -> SectorDeal { SectorDeal:: { sector_number: self.sector_number, @@ -648,12 +665,7 @@ fn verify_deals_for_activation_fails_sector_activation_after_start_block() { // wait a couple of blocks so deal cannot be activated, because it's too late. run_to_block(2); - publish_for_activation( - 1, - DealProposalBuilder::default() - .start_block(1) - .build(), - ); + publish_for_activation(1, DealProposalBuilder::default().start_block(1).build()); let deals = bounded_vec![SectorDealBuilder::default().build()]; @@ -684,24 +696,127 @@ fn verify_deals_for_activation_fails_sector_expires_before_deal_ends() { }); } +#[test] +fn verify_deals_for_activation_fails_not_enough_space() { + new_test_ext().execute_with(|| { + publish_for_activation( + 1, + DealProposalBuilder::default() + .piece_size(1 << 10 /* 1 KiB */) + .build(), + ); + publish_for_activation( + 2, + DealProposalBuilder::default() + .piece_size(3 << 10 /* 3 KiB */) + .build(), + ); + // 1 KiB + 3KiB >= 2 KiB (sector size) + + let deals = bounded_vec![SectorDealBuilder::default() + .deal_ids(bounded_vec![1, 2]) + .build()]; + + assert_noop!( + Market::verify_deals_for_activation(&account(PROVIDER), deals), + Error::::DealsTooLargeToFitIntoSector + ); + }); +} + +#[test] +fn verify_deals_for_activation_fails_duplicate_deals() { + new_test_ext().execute_with(|| { + publish_for_activation(1, DealProposalBuilder::default().build()); + + let deals = bounded_vec![SectorDealBuilder::default() + .deal_ids(bounded_vec![1, 1]) + .build()]; + + assert_noop!( + Market::verify_deals_for_activation(&account(PROVIDER), deals), + Error::::DuplicateDeal + ); + }); +} + +#[test] +fn verify_deals_for_activation_fails_deal_not_found() { + new_test_ext().execute_with(|| { + let deals = bounded_vec![SectorDealBuilder::default() + .deal_ids(bounded_vec![1, 2, 3, 4]) + .build()]; + + assert_noop!( + Market::verify_deals_for_activation(&account(PROVIDER), deals), + Error::::DealNotFound + ); + }); +} + #[test] fn activate_deals() { new_test_ext().execute_with(|| { let alice_hash = publish_for_activation(1, DealProposalBuilder::default().build()); let deals = bounded_vec![ - SectorDeal { - sector_number: 1, - sector_expiry: 120, - sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, - deal_ids: bounded_vec![1] - }, - SectorDeal { - sector_number: 2, - sector_expiry: 50, - sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, - deal_ids: bounded_vec![] - } + SectorDealBuilder::default().build(), + SectorDealBuilder::default() + .sector_number(2) + .sector_expiry(50) + .deal_ids(bounded_vec![]) + .build() + ]; + + let piece_cid = + Cid::from_str("bafk2bzacecg3xxc4f2ql2hreiuy767u6r72ekdz54k7luieknboaakhft5rgk") + .unwrap(); + let placeholder_commd_cid = + Cid::from_str("bafk2bzaceajreoxfdcpdvitpvxm7vkpvcimlob5ejebqgqidjkz4qoug4q6zu") + .unwrap(); + assert_eq!( + Ok(bounded_vec![ + ActiveSector { + active_deals: bounded_vec![ActiveDeal { + client: account(ALICE), + piece_cid: piece_cid, + piece_size: 18 + }], + unsealed_cid: Some(placeholder_commd_cid), + }, + ActiveSector { + active_deals: bounded_vec![], + unsealed_cid: None + } + ]), + Market::activate_deals(&account(PROVIDER), deals, true) + ); + assert!(!PendingProposals::::get().contains(&alice_hash)); + }); +} + +#[test] +fn activate_deals_fails_for_1_sector_but_succeeds_for_others() { + new_test_ext().execute_with(|| { + let alice_hash = publish_for_activation(1, DealProposalBuilder::default().build()); + let _ = publish_for_activation(2, DealProposalBuilder::default().build()); + let deals = bounded_vec![ + SectorDealBuilder::default().build(), + SectorDealBuilder::default() + .sector_number(2) + .sector_expiry(50) + .deal_ids(bounded_vec![]) + .build(), + SectorDealBuilder::default() + .sector_number(3) + .deal_ids(bounded_vec![1337]) + .build(), + SectorDealBuilder::default() + .sector_number(4) + // force error by making expiry < start_block < end_block + .sector_expiry(10) + .deal_ids(bounded_vec![2]) + .build() ]; let piece_cid = From 2ce0881d7113f62c755c34f2ff87ba024c0467d7 Mon Sep 17 00:00:00 2001 From: Konrad Stepniak Date: Thu, 4 Jul 2024 09:55:13 +0200 Subject: [PATCH 07/16] test(pallet-market): reorganize builders to be at the end of the file --- pallets/market/src/test.rs | 96 +++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/pallets/market/src/test.rs b/pallets/market/src/test.rs index 0f79bc15c..750ba1c47 100644 --- a/pallets/market/src/test.rs +++ b/pallets/market/src/test.rs @@ -558,54 +558,6 @@ fn verify_deals_for_activation() { }); } -/// Builder with nice defaults for test purposes. -struct SectorDealBuilder { - sector_number: u64, - sector_expiry: u64, - sector_type: RegisteredSealProof, - deal_ids: BoundedVec>, -} - -impl SectorDealBuilder { - pub fn sector_expiry(mut self, sector_expiry: u64) -> Self { - self.sector_expiry = sector_expiry; - self - } - - pub fn sector_number(mut self, sector_number: u64) -> Self { - self.sector_number = sector_number; - self - } - - pub fn deal_ids( - mut self, - deal_ids: BoundedVec>, - ) -> Self { - self.deal_ids = deal_ids; - self - } - - pub fn build(self) -> SectorDeal { - SectorDeal:: { - sector_number: self.sector_number, - sector_expiry: self.sector_expiry, - sector_type: self.sector_type, - deal_ids: self.deal_ids, - } - } -} - -impl Default for SectorDealBuilder { - fn default() -> Self { - Self { - sector_number: 1, - sector_expiry: 120, - sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, - deal_ids: bounded_vec![1], - } - } -} - #[test] fn verify_deals_for_activation_fails_with_different_provider() { new_test_ext().execute_with(|| { @@ -1396,3 +1348,51 @@ impl DealProposalBuilder { signed } } + +/// Builder with nice defaults for test purposes. +struct SectorDealBuilder { + sector_number: u64, + sector_expiry: u64, + sector_type: RegisteredSealProof, + deal_ids: BoundedVec>, +} + +impl SectorDealBuilder { + pub fn sector_expiry(mut self, sector_expiry: u64) -> Self { + self.sector_expiry = sector_expiry; + self + } + + pub fn sector_number(mut self, sector_number: u64) -> Self { + self.sector_number = sector_number; + self + } + + pub fn deal_ids( + mut self, + deal_ids: BoundedVec>, + ) -> Self { + self.deal_ids = deal_ids; + self + } + + pub fn build(self) -> SectorDeal { + SectorDeal:: { + sector_number: self.sector_number, + sector_expiry: self.sector_expiry, + sector_type: self.sector_type, + deal_ids: self.deal_ids, + } + } +} + +impl Default for SectorDealBuilder { + fn default() -> Self { + Self { + sector_number: 1, + sector_expiry: 120, + sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, + deal_ids: bounded_vec![1], + } + } +} From 922dbfc64b8d89339f2337699ae8a934ba35fbc6 Mon Sep 17 00:00:00 2001 From: Konrad Stepniak Date: Thu, 4 Jul 2024 10:18:44 +0200 Subject: [PATCH 08/16] style(pallet-market): format rust-toolchain --- rust-toolchain.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 9ed71b738..7a9b7b4e3 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,5 +1,5 @@ [toolchain] channel = "1.77.0" -components = ["cargo", "clippy", "rust-analyzer", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt", "llvm-tools-preview"] +components = ["cargo", "clippy", "llvm-tools-preview", "rust-analyzer", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] profile = "minimal" targets = ["wasm32-unknown-unknown"] From 17b4c12b22b762768f005c01c22ff6072b84e1b7 Mon Sep 17 00:00:00 2001 From: th7nder Date: Fri, 5 Jul 2024 15:13:54 +0200 Subject: [PATCH 09/16] test(pallet-market): simplify tests --- pallets/market/src/test.rs | 92 +++++++++++--------------------------- 1 file changed, 26 insertions(+), 66 deletions(-) diff --git a/pallets/market/src/test.rs b/pallets/market/src/test.rs index 729769b03..a74e11cd7 100644 --- a/pallets/market/src/test.rs +++ b/pallets/market/src/test.rs @@ -6,7 +6,7 @@ use frame_support::{ pallet_prelude::ConstU32, sp_runtime::{bounded_vec, ArithmeticError, DispatchError, TokenError}, traits::Currency, - BoundedBTreeSet, BoundedVec, + BoundedVec, }; use primitives_proofs::{ ActiveDeal, ActiveSector, DealId, Market as MarketTrait, RegisteredSealProof, SectorDeal, @@ -17,8 +17,8 @@ use sp_core::H256; use crate::{ mock::*, pallet::{lock_funds, slash_and_burn, unlock_funds}, - ActiveDealState, BalanceEntry, BalanceTable, DealProposal, DealSettlementError, DealState, - DealsForBlock, Error, Event, PendingProposals, Proposals, SectorDeals, SectorTerminateError, + ActiveDealState, BalanceEntry, BalanceTable, DealSettlementError, DealState, DealsForBlock, + Error, Event, PendingProposals, Proposals, SectorDeals, SectorTerminateError, }; #[test] fn initial_state() { @@ -984,21 +984,13 @@ fn settle_deal_payments_published() { Proposals::::insert( 1, - DealProposal { - piece_cid: cid_of("polka-storage-data-bob") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 21, - client: account(BOB), - provider: account(PROVIDER), - label: bounded_vec![0xa, 0xe, 0xe, 0xf], - start_block: 0, - end_block: 10, - storage_price_per_block: 10, - provider_collateral: 15, - state: DealState::Published, - }, + DealProposalBuilder::default() + .client(BOB) + .start_block(0) + .end_block(10) + .storage_price_per_block(10) + .provider_collateral(15) + .build(), ); System::reset_events(); @@ -1614,24 +1606,7 @@ fn on_sector_terminate_invalid_caller() { let sector_deal_ids: BoundedVec<_, ConstU32> = bounded_vec![1]; SectorDeals::::insert(cid.clone(), sector_deal_ids); - Proposals::::insert( - 1, - DealProposal { - piece_cid: cid_of("polka-storage-data-bob") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 21, - client: account(BOB), - provider: account(PROVIDER), - label: bounded_vec![0xa, 0xe, 0xe, 0xf], - start_block: 0, - end_block: 10, - storage_price_per_block: 10, - provider_collateral: 15, - state: DealState::Published, - }, - ); + Proposals::::insert(1, DealProposalBuilder::default().client(BOB).build()); assert_err!( Market::on_sectors_terminate(&account(BOB), bounded_vec![cid],), @@ -1655,21 +1630,13 @@ fn on_sector_terminate_not_active() { SectorDeals::::insert(cid.clone(), sector_deal_ids); Proposals::::insert( 1, - DealProposal { - piece_cid: cid_of("polka-storage-data-bob") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 21, - client: account(BOB), - provider: account(PROVIDER), - label: bounded_vec![0xa, 0xe, 0xe, 0xf], - start_block: 0, - end_block: 10, - storage_price_per_block: 10, - provider_collateral: 15, - state: DealState::Published, - }, + DealProposalBuilder::default() + .client(BOB) + .start_block(0) + .end_block(10) + .storage_price_per_block(10) + .provider_collateral(15) + .build(), ); assert_err!( @@ -1690,21 +1657,14 @@ fn on_sector_terminate_active() { let cid = BoundedVec::try_from(cid_of("polka_storage_cid").to_bytes()).unwrap(); let sector_deal_ids: BoundedVec<_, ConstU32> = bounded_vec![1]; - let deal_proposal = DealProposal { - piece_cid: cid_of("polka_storage_piece") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 21, - client: account(BOB), - provider: account(PROVIDER), - label: bounded_vec![0xa, 0xe, 0xe, 0xf], - start_block: 0, - end_block: 10, - storage_price_per_block: 5, - provider_collateral: 15, - state: DealState::Active(ActiveDealState::new(0, 0)), - }; + let deal_proposal = DealProposalBuilder::default() + .client(BOB) + .start_block(0) + .end_block(10) + .storage_price_per_block(5) + .provider_collateral(15) + .state(DealState::Active(ActiveDealState::new(0, 0))) + .build(); assert_ok!(lock_funds::(&account(BOB), 5 * 10)); assert_ok!(lock_funds::(&account(PROVIDER), 15)); From ccdd2712741263421d3df25fbaf0d7a9087619a3 Mon Sep 17 00:00:00 2001 From: th7nder Date: Fri, 5 Jul 2024 15:16:33 +0200 Subject: [PATCH 10/16] style(pallet-market): reformat --- pallets/market/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pallets/market/src/lib.rs b/pallets/market/src/lib.rs index f43700162..43bf380ef 100644 --- a/pallets/market/src/lib.rs +++ b/pallets/market/src/lib.rs @@ -31,8 +31,7 @@ pub mod pallet { traits::{ Currency, ExistenceRequirement::{AllowDeath, KeepAlive}, - Hooks, ReservableCurrency, - WithdrawReasons + Hooks, ReservableCurrency, WithdrawReasons, }, PalletId, }; From a3e338de3fc347ee287897c1f6ab0ead016df7c8 Mon Sep 17 00:00:00 2001 From: th7nder Date: Sat, 6 Jul 2024 22:44:28 +0200 Subject: [PATCH 11/16] build: remove crates from .vscode/extensions --- .vscode/extensions.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/.vscode/extensions.json b/.vscode/extensions.json index 20339f51d..bbb438c90 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -10,7 +10,5 @@ "ryanluker.vscode-coverage-gutters", // direnv, to use tool binaries from `nix` profile and set environment variables automatically "mkhl.direnv", - // Inline information about crates - "serayuzgur.crates" ] } \ No newline at end of file From e69ba5b9f172b6b36cc3d135662961365b2058eb Mon Sep 17 00:00:00 2001 From: th7nder Date: Sat, 6 Jul 2024 22:56:38 +0200 Subject: [PATCH 12/16] tests(pallet-market): simplify and make more maintainable --- pallets/market/src/lib.rs | 2 +- pallets/market/src/test.rs | 338 ++++++++++++++++++------------------- 2 files changed, 170 insertions(+), 170 deletions(-) diff --git a/pallets/market/src/lib.rs b/pallets/market/src/lib.rs index 43bf380ef..12f4f1f71 100644 --- a/pallets/market/src/lib.rs +++ b/pallets/market/src/lib.rs @@ -1133,7 +1133,7 @@ pub mod pallet { let mut pending_proposals = PendingProposals::::get(); for sector in sector_deals { let Ok(proposals) = Self::proposals_for_deals(sector.deal_ids) else { - log::error!("failed to find deals for sector: {}", sector.sector_number,); + log::error!("failed to find deals for sector: {}", sector.sector_number); continue; }; diff --git a/pallets/market/src/test.rs b/pallets/market/src/test.rs index a74e11cd7..32627f867 100644 --- a/pallets/market/src/test.rs +++ b/pallets/market/src/test.rs @@ -535,7 +535,7 @@ fn publish_storage_deals() { #[test] fn verify_deals_for_activation() { new_test_ext().execute_with(|| { - publish_for_activation(1, DealProposalBuilder::default().build()); + publish_for_activation(1, DealProposalBuilder::default().unsigned()); let deals = bounded_vec![ SectorDeal { @@ -568,7 +568,7 @@ fn verify_deals_for_activation() { #[test] fn verify_deals_for_activation_fails_with_different_provider() { new_test_ext().execute_with(|| { - publish_for_activation(1, DealProposalBuilder::default().provider(BOB).build()); + publish_for_activation(1, DealProposalBuilder::default().provider(BOB).unsigned()); let deals = bounded_vec![SectorDealBuilder::default().build()]; @@ -591,7 +591,7 @@ fn verify_deals_for_activation_fails_with_invalid_deal_state() { last_updated_block: Some(10), slash_block: None, })) - .build(), + .unsigned(), ); let deals = bounded_vec![SectorDealBuilder::default().build()]; @@ -607,7 +607,7 @@ fn verify_deals_for_activation_fails_with_invalid_deal_state() { fn verify_deals_for_activation_fails_deal_not_in_pending() { new_test_ext().execute_with(|| { // do not use `publish_for_activation` as it puts deal in PendingProposals - Proposals::::insert(1, DealProposalBuilder::default().build()); + Proposals::::insert(1, DealProposalBuilder::default().unsigned()); let deals = bounded_vec![SectorDealBuilder::default().build()]; assert_noop!( @@ -624,7 +624,7 @@ fn verify_deals_for_activation_fails_sector_activation_after_start_block() { // wait a couple of blocks so deal cannot be activated, because it's too late. run_to_block(2); - publish_for_activation(1, DealProposalBuilder::default().start_block(1).build()); + publish_for_activation(1, DealProposalBuilder::default().start_block(1).unsigned()); let deals = bounded_vec![SectorDealBuilder::default().build()]; @@ -643,7 +643,7 @@ fn verify_deals_for_activation_fails_sector_expires_before_deal_ends() { DealProposalBuilder::default() .start_block(10) .end_block(15) - .build(), + .unsigned(), ); let deals = bounded_vec![SectorDealBuilder::default().sector_expiry(11).build()]; @@ -662,13 +662,13 @@ fn verify_deals_for_activation_fails_not_enough_space() { 1, DealProposalBuilder::default() .piece_size(1 << 10 /* 1 KiB */) - .build(), + .unsigned(), ); publish_for_activation( 2, DealProposalBuilder::default() .piece_size(3 << 10 /* 3 KiB */) - .build(), + .unsigned(), ); // 1 KiB + 3KiB >= 2 KiB (sector size) @@ -686,7 +686,7 @@ fn verify_deals_for_activation_fails_not_enough_space() { #[test] fn verify_deals_for_activation_fails_duplicate_deals() { new_test_ext().execute_with(|| { - publish_for_activation(1, DealProposalBuilder::default().build()); + publish_for_activation(1, DealProposalBuilder::default().unsigned()); let deals = bounded_vec![SectorDealBuilder::default() .deal_ids(bounded_vec![1, 1]) @@ -716,7 +716,7 @@ fn verify_deals_for_activation_fails_deal_not_found() { #[test] fn activate_deals() { new_test_ext().execute_with(|| { - let alice_hash = publish_for_activation(1, DealProposalBuilder::default().build()); + let alice_hash = publish_for_activation(1, DealProposalBuilder::default().unsigned()); let deals = bounded_vec![ SectorDealBuilder::default().build(), @@ -757,8 +757,8 @@ fn activate_deals() { #[test] fn activate_deals_fails_for_1_sector_but_succeeds_for_others() { new_test_ext().execute_with(|| { - let alice_hash = publish_for_activation(1, DealProposalBuilder::default().build()); - let _ = publish_for_activation(2, DealProposalBuilder::default().build()); + let alice_hash = publish_for_activation(1, DealProposalBuilder::default().unsigned()); + let _ = publish_for_activation(2, DealProposalBuilder::default().unsigned()); let deals = bounded_vec![ SectorDealBuilder::default().build(), SectorDealBuilder::default() @@ -772,7 +772,7 @@ fn activate_deals_fails_for_1_sector_but_succeeds_for_others() { .build(), SectorDealBuilder::default() .sector_number(4) - // force error by making expiry < start_block < end_block + // force error by making expiry < start_block .sector_expiry(10) .deal_ids(bounded_vec![2]) .build() @@ -990,7 +990,7 @@ fn settle_deal_payments_published() { .end_block(10) .storage_price_per_block(10) .provider_collateral(15) - .build(), + .unsigned(), ); System::reset_events(); @@ -1027,7 +1027,7 @@ fn settle_deal_payments_active_future_last_update() { last_updated_block: Some(10), slash_block: None, })) - .build(), + .unsigned(), ); System::reset_events(); @@ -1063,7 +1063,7 @@ fn settle_deal_payments_active_corruption() { last_updated_block: Some(11), slash_block: None, })) - .build(), + .unsigned(), ); run_to_block(12); System::reset_events(); @@ -1116,7 +1116,7 @@ fn settle_deal_payments_success() { last_updated_block: None, slash_block: None, })) - .build() + .unsigned() ) ); System::reset_events(); @@ -1164,7 +1164,7 @@ fn settle_deal_payments_success() { last_updated_block: Some(5), slash_block: None, })) - .build() + .unsigned() ) ); }); @@ -1209,7 +1209,7 @@ fn settle_deal_payments_success_finished() { last_updated_block: None, slash_block: None, })) - .build() + .unsigned() ) ); @@ -1251,153 +1251,6 @@ fn settle_deal_payments_success_finished() { }); } -/// Builder to simplify writing complex tests of [`DealProposal`] -/// Use of Generics is strictly limited and concrete values for the Test config are used instead. -struct DealProposalBuilder { - piece_cid: BoundedVec>, - piece_size: u64, - client: AccountIdOf, - provider: AccountIdOf, - label: BoundedVec>, - start_block: u64, - end_block: u64, - storage_price_per_block: u64, - provider_collateral: u64, - state: DealState, -} - -impl Default for DealProposalBuilder { - fn default() -> Self { - Self { - piece_cid: cid_of("polka-storage-data") - .to_bytes() - .try_into() - .expect("hash is always 32 bytes"), - piece_size: 18, - client: account(ALICE), - provider: account(PROVIDER), - label: bounded_vec![0xb, 0xe, 0xe, 0xf], - start_block: 100, - end_block: 110, - storage_price_per_block: 5, - provider_collateral: 25, - // TODO(@th7nder,01/07/2024): change this to Published - state: DealState::Published, - } - } -} - -impl DealProposalBuilder { - pub fn client(mut self, client: &'static str) -> Self { - self.client = account(client); - self - } - - pub fn provider(mut self, provider: &'static str) -> Self { - self.provider = account(provider); - self - } - - pub fn state(mut self, state: DealState) -> Self { - self.state = state; - self - } - - pub fn start_block(mut self, start_block: u64) -> Self { - self.start_block = start_block; - self - } - - pub fn end_block(mut self, end_block: u64) -> Self { - self.end_block = end_block; - self - } - - pub fn storage_price_per_block(mut self, price: u64) -> Self { - self.storage_price_per_block = price; - self - } - - pub fn provider_collateral(mut self, price: u64) -> Self { - self.provider_collateral = price; - self - } - - pub fn piece_size(mut self, piece_size: u64) -> Self { - self.piece_size = piece_size; - self - } - - pub fn build(self) -> DealProposalOf { - DealProposalOf:: { - piece_cid: self.piece_cid, - piece_size: self.piece_size, - client: self.client, - provider: self.provider, - label: self.label, - start_block: self.start_block, - end_block: self.end_block, - storage_price_per_block: self.storage_price_per_block, - provider_collateral: self.provider_collateral, - state: self.state, - } - } - - pub fn signed(self, by: &'static str) -> ClientDealProposalOf { - let built = self.build(); - let signed = sign_proposal(by, built); - signed - } -} - -/// Builder with nice defaults for test purposes. -struct SectorDealBuilder { - sector_number: u64, - sector_expiry: u64, - sector_type: RegisteredSealProof, - deal_ids: BoundedVec>, -} - -impl SectorDealBuilder { - pub fn sector_expiry(mut self, sector_expiry: u64) -> Self { - self.sector_expiry = sector_expiry; - self - } - - pub fn sector_number(mut self, sector_number: u64) -> Self { - self.sector_number = sector_number; - self - } - - pub fn deal_ids( - mut self, - deal_ids: BoundedVec>, - ) -> Self { - self.deal_ids = deal_ids; - self - } - - pub fn build(self) -> SectorDeal { - SectorDeal:: { - sector_number: self.sector_number, - sector_expiry: self.sector_expiry, - sector_type: self.sector_type, - deal_ids: self.deal_ids, - } - } -} - -impl Default for SectorDealBuilder { - fn default() -> Self { - Self { - sector_number: 1, - sector_expiry: 120, - sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, - deal_ids: bounded_vec![1], - } - } -} - #[test] fn test_lock_funds() { let _ = env_logger::try_init(); @@ -1606,7 +1459,7 @@ fn on_sector_terminate_invalid_caller() { let sector_deal_ids: BoundedVec<_, ConstU32> = bounded_vec![1]; SectorDeals::::insert(cid.clone(), sector_deal_ids); - Proposals::::insert(1, DealProposalBuilder::default().client(BOB).build()); + Proposals::::insert(1, DealProposalBuilder::default().client(BOB).unsigned()); assert_err!( Market::on_sectors_terminate(&account(BOB), bounded_vec![cid],), @@ -1636,7 +1489,7 @@ fn on_sector_terminate_not_active() { .end_block(10) .storage_price_per_block(10) .provider_collateral(15) - .build(), + .unsigned(), ); assert_err!( @@ -1664,7 +1517,7 @@ fn on_sector_terminate_active() { .storage_price_per_block(5) .provider_collateral(15) .state(DealState::Active(ActiveDealState::new(0, 0))) - .build(); + .unsigned(); assert_ok!(lock_funds::(&account(BOB), 5 * 10)); assert_ok!(lock_funds::(&account(PROVIDER), 15)); @@ -1724,3 +1577,150 @@ fn on_sector_terminate_active() { ); }); } + +/// Builder with nice defaults for test purposes. +struct SectorDealBuilder { + sector_number: u64, + sector_expiry: u64, + sector_type: RegisteredSealProof, + deal_ids: BoundedVec>, +} + +impl SectorDealBuilder { + pub fn sector_expiry(mut self, sector_expiry: u64) -> Self { + self.sector_expiry = sector_expiry; + self + } + + pub fn sector_number(mut self, sector_number: u64) -> Self { + self.sector_number = sector_number; + self + } + + pub fn deal_ids( + mut self, + deal_ids: BoundedVec>, + ) -> Self { + self.deal_ids = deal_ids; + self + } + + pub fn build(self) -> SectorDeal { + SectorDeal:: { + sector_number: self.sector_number, + sector_expiry: self.sector_expiry, + sector_type: self.sector_type, + deal_ids: self.deal_ids, + } + } +} + +impl Default for SectorDealBuilder { + fn default() -> Self { + Self { + sector_number: 1, + sector_expiry: 120, + sector_type: RegisteredSealProof::StackedDRG2KiBV1P1, + deal_ids: bounded_vec![1], + } + } +} + +/// Builder to simplify writing complex tests of [`DealProposal`]. +/// Exclusively uses [`Test`] for simplification purposes. +struct DealProposalBuilder { + piece_cid: BoundedVec>, + piece_size: u64, + client: AccountIdOf, + provider: AccountIdOf, + label: BoundedVec>, + start_block: u64, + end_block: u64, + storage_price_per_block: u64, + provider_collateral: u64, + state: DealState, +} + +impl Default for DealProposalBuilder { + fn default() -> Self { + Self { + piece_cid: cid_of("polka-storage-data") + .to_bytes() + .try_into() + .expect("hash is always 32 bytes"), + piece_size: 18, + client: account(ALICE), + provider: account(PROVIDER), + label: bounded_vec![0xb, 0xe, 0xe, 0xf], + start_block: 100, + end_block: 110, + storage_price_per_block: 5, + provider_collateral: 25, + // TODO(@th7nder,01/07/2024): change this to Published + state: DealState::Published, + } + } +} + +impl DealProposalBuilder { + pub fn client(mut self, client: &'static str) -> Self { + self.client = account(client); + self + } + + pub fn provider(mut self, provider: &'static str) -> Self { + self.provider = account(provider); + self + } + + pub fn state(mut self, state: DealState) -> Self { + self.state = state; + self + } + + pub fn start_block(mut self, start_block: u64) -> Self { + self.start_block = start_block; + self + } + + pub fn end_block(mut self, end_block: u64) -> Self { + self.end_block = end_block; + self + } + + pub fn storage_price_per_block(mut self, price: u64) -> Self { + self.storage_price_per_block = price; + self + } + + pub fn provider_collateral(mut self, price: u64) -> Self { + self.provider_collateral = price; + self + } + + pub fn piece_size(mut self, piece_size: u64) -> Self { + self.piece_size = piece_size; + self + } + + pub fn unsigned(self) -> DealProposalOf { + DealProposalOf:: { + piece_cid: self.piece_cid, + piece_size: self.piece_size, + client: self.client, + provider: self.provider, + label: self.label, + start_block: self.start_block, + end_block: self.end_block, + storage_price_per_block: self.storage_price_per_block, + provider_collateral: self.provider_collateral, + state: self.state, + } + } + + pub fn signed(self, by: &'static str) -> ClientDealProposalOf { + let built = self.unsigned(); + let signed = sign_proposal(by, built); + signed + } +} From 4c50400e91188f51ad0e5ea1447b3a801421096b Mon Sep 17 00:00:00 2001 From: th7nder Date: Sat, 6 Jul 2024 22:59:20 +0200 Subject: [PATCH 13/16] build: remove rust bundle --- .vscode/extensions.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.vscode/extensions.json b/.vscode/extensions.json index bbb438c90..a070ed80a 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -1,7 +1,7 @@ { "recommendations": [ // Rust language support - "1yib.rust-bundle", + "rust-lang.rust-analyzer", // Justfile "nefrob.vscode-just-syntax", // TOML support From 0c1d0b35fdfa5f9807c226c1d59e7ba07da2c54b Mon Sep 17 00:00:00 2001 From: Konrad Stepniak Date: Mon, 8 Jul 2024 15:50:47 +0000 Subject: [PATCH 14/16] test(pallet-market): make fails min duration more readable --- pallets/market/src/test.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pallets/market/src/test.rs b/pallets/market/src/test.rs index 32627f867..5dbb5834d 100644 --- a/pallets/market/src/test.rs +++ b/pallets/market/src/test.rs @@ -3,9 +3,9 @@ use core::str::FromStr; use cid::Cid; use frame_support::{ assert_err, assert_noop, assert_ok, - pallet_prelude::ConstU32, + pallet_prelude::{ConstU32,Get}, sp_runtime::{bounded_vec, ArithmeticError, DispatchError, TokenError}, - traits::Currency, + traits::{Currency}, BoundedVec, }; use primitives_proofs::{ @@ -18,7 +18,7 @@ use crate::{ mock::*, pallet::{lock_funds, slash_and_burn, unlock_funds}, ActiveDealState, BalanceEntry, BalanceTable, DealSettlementError, DealState, DealsForBlock, - Error, Event, PendingProposals, Proposals, SectorDeals, SectorTerminateError, + Error, Event, Config, PendingProposals, Proposals, SectorDeals, SectorTerminateError, }; #[test] fn initial_state() { @@ -273,8 +273,7 @@ fn publish_storage_deals_fails_min_duration_out_of_bounds() { new_test_ext().execute_with(|| { let proposal = DealProposalBuilder::default() .start_block(10) - // Make duration shorter than [`T::MinDealDuration`] - .end_block(11) + .end_block(10 + <::MinDealDuration as Get>::get() - 1) .signed(ALICE); assert_noop!( From 4ee45f7926e8ff6106f142b57ba9eecc15392242 Mon Sep 17 00:00:00 2001 From: Konrad Stepniak Date: Mon, 8 Jul 2024 16:03:16 +0000 Subject: [PATCH 15/16] test(pallet-market): make em more readable --- pallets/market/src/test.rs | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/pallets/market/src/test.rs b/pallets/market/src/test.rs index 5dbb5834d..16558f7d0 100644 --- a/pallets/market/src/test.rs +++ b/pallets/market/src/test.rs @@ -3,9 +3,9 @@ use core::str::FromStr; use cid::Cid; use frame_support::{ assert_err, assert_noop, assert_ok, - pallet_prelude::{ConstU32,Get}, + pallet_prelude::{ConstU32, Get}, sp_runtime::{bounded_vec, ArithmeticError, DispatchError, TokenError}, - traits::{Currency}, + traits::Currency, BoundedVec, }; use primitives_proofs::{ @@ -17,8 +17,8 @@ use sp_core::H256; use crate::{ mock::*, pallet::{lock_funds, slash_and_burn, unlock_funds}, - ActiveDealState, BalanceEntry, BalanceTable, DealSettlementError, DealState, DealsForBlock, - Error, Event, Config, PendingProposals, Proposals, SectorDeals, SectorTerminateError, + ActiveDealState, BalanceEntry, BalanceTable, Config, DealSettlementError, DealState, + DealsForBlock, Error, Event, PendingProposals, Proposals, SectorDeals, SectorTerminateError, }; #[test] fn initial_state() { @@ -290,8 +290,8 @@ fn publish_storage_deals_fails_min_duration_out_of_bounds() { fn publish_storage_deals_fails_max_duration_out_of_bounds() { new_test_ext().execute_with(|| { let proposal = DealProposalBuilder::default() - // Make duration too many blocks. - .end_block(1000) + .start_block(100) + .end_block(100 + <::MaxDealDuration as Get>::get() + 1) .signed(ALICE); assert_noop!( @@ -304,11 +304,11 @@ fn publish_storage_deals_fails_max_duration_out_of_bounds() { }); } +/// Add enough balance to the provider so that the first proposal can be accepted and published. +/// Second proposal will be rejected, but first still published #[test] fn publish_storage_deals_fails_different_providers() { new_test_ext().execute_with(|| { - // Let the first proposal pass, by adding enough balance for Alice - // Second proposal will be rejected, but first still published let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 60); let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 60); System::reset_events(); @@ -335,11 +335,11 @@ fn publish_storage_deals_fails_different_providers() { }); } +/// Add enough balance to the provider so that the first proposal can be accepted and published. +/// Second proposal will be rejected, but first still published #[test] fn publish_storage_deals_fails_client_not_enough_funds_for_second_deal() { new_test_ext().execute_with(|| { - // Let the first proposal pass, by adding enough balance for Alice - // Second proposal will be rejected, but first still published let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 60); let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 60); System::reset_events(); @@ -348,7 +348,7 @@ fn publish_storage_deals_fails_client_not_enough_funds_for_second_deal() { RuntimeOrigin::signed(account(PROVIDER)), bounded_vec![ DealProposalBuilder::default().signed(ALICE), - DealProposalBuilder::default().signed(ALICE), + DealProposalBuilder::default().piece_size(10).signed(ALICE), ] )); assert_eq!( @@ -362,12 +362,12 @@ fn publish_storage_deals_fails_client_not_enough_funds_for_second_deal() { }); } +/// Add enough balance to the provider so that the first proposal can be accepted and published. +/// Collateral is 25 for the default deal, so provider should have at least 50. +/// Second proposal will be rejected, but first still published #[test] fn publish_storage_deals_fails_provider_not_enough_funds_for_second_deal() { new_test_ext().execute_with(|| { - // Let the first proposal pass, by adding enough balance for Provider - // Collateral is 25 for the default deal, so provider should have at least 50. - // Second proposal will be rejected, but first still published let _ = Market::add_balance(RuntimeOrigin::signed(account(PROVIDER)), 40); let _ = Market::add_balance(RuntimeOrigin::signed(account(ALICE)), 90); let _ = Market::add_balance(RuntimeOrigin::signed(account(BOB)), 90); @@ -481,11 +481,7 @@ fn publish_storage_deals() { assert_ok!(Market::publish_storage_deals( RuntimeOrigin::signed(account(PROVIDER)), - bounded_vec![ - alice_proposal.clone(), - alice_second_proposal.clone(), - bob_proposal.clone() - ] + bounded_vec![alice_proposal, alice_second_proposal, bob_proposal] )); assert_eq!( BalanceTable::::get(account(ALICE)), From f8643e9f8c5bec1cf02de68c962285d3dfe12b20 Mon Sep 17 00:00:00 2001 From: Konrad Stepniak Date: Mon, 8 Jul 2024 16:20:50 +0000 Subject: [PATCH 16/16] test(pallet-market): deal from the past --- pallets/market/src/test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/market/src/test.rs b/pallets/market/src/test.rs index 16558f7d0..f3eb15243 100644 --- a/pallets/market/src/test.rs +++ b/pallets/market/src/test.rs @@ -613,7 +613,7 @@ fn verify_deals_for_activation_fails_deal_not_in_pending() { } #[test] -fn verify_deals_for_activation_fails_sector_activation_after_start_block() { +fn verify_deals_for_activation_fails_sector_activation_on_deal_from_the_past() { new_test_ext().execute_with(|| { // current_block == sector_activation when calling `verify_deals_for_activation` // wait a couple of blocks so deal cannot be activated, because it's too late.