[monero-project/monero] Bp multi aggregation pippenger (#4219)

stoffu commented on this pull request.

Some comments on core tests

> + ++amounts_paid;
+ ++amounts_paid;
+ }
+ if (!valid)
+ DO_CALLBACK(events, «mark_invalid_tx»);
+ events.push_back(rct_txes);
+
+ CHECK_AND_ASSERT_MES(generator.construct_block_manually(blk_txes, blk_last, miner_account,
+ test_generator::bf_major_ver | test_generator::bf_minor_ver | test_generator::bf_timestamp | test_generator::bf_tx_hashes | test_generator::bf_hf_version | test_generator::bf_max_outs,
+ 8, 8, blk_last.timestamp + DIFFICULTY_BLOCKS_ESTIMATE_TIMESPAN * 2, // v2 has blocks twice as long
+ crypto::hash(), 0, transaction(), starting_rct_tx_hashes, 0, 6, 8),
+ false, «Failed to generate block»);
+ if (!valid)
+ DO_CALLBACK(events, «mark_invalid_block»);
+ events.push_back(blk_txes);
+ blk_last = blk_txes;

Isn’t this a no-op?

> +}
+
+bool gen_bp_tx_valid_16::generate(std::vector& events) const
+{
+ const size_t mixin = 10;
+ const uint64_t amounts_paid[] = {500, 500, 500, 500, 500, 500, 500, 500, 500, 500, 500, 500, 500, 500, 500, 500, (uint64_t)-1};
+ const size_t bp_sizes[] = {16, (size_t)-1};
+ const rct::RangeProofType range_proof_type[] = { rct::RangeProofPaddedBulletproof };
+ return generate_with(events, mixin, 1, amounts_paid, true, range_proof_type, NULL, [&](const cryptonote::transaction &tx, size_t tx_idx){ return check_bp(tx, tx_idx, bp_sizes, «gen_bp_tx_valid_16»); });
+}
+
+bool gen_bp_tx_invalid_4_2_1::generate(std::vector& events) const
+{
+ const size_t mixin = 10;
+ const uint64_t amounts_paid[] = {1000, 1000, 1000, 1000, 1000, 1000, 1000, (uint64_t)-1};
+ const size_t bp_sizes[] = {4, 2, 1, (size_t)-1};

The `bp_sizes` here and below is unused when the `post_tx` arg is set to null. Maybe drop it for the invalid tests, and bring in the `post_tx` with `check_bp` for the valid tests, namely `gen_bp_txs_valid_2_and_2` and `gen_bp_txs_valid_2_and_3_and_2_and_4`?

> +
+bool gen_bp_txs_invalid_2_and_8_2_and_16_16_1::generate(std::vector& events) const
+{
+ const size_t mixin = 10;
+ const uint64_t amounts_paid[] = {1000, 1000, (uint64_t)-1, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, (uint64_t)-1, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, 1000, (uint64_t)-1};
+ const rct::RangeProofType range_proof_type[] = {rct::RangeProofMultiOutputBulletproof, rct::RangeProofMultiOutputBulletproof, rct::RangeProofMultiOutputBulletproof};
+ const size_t bp_sizes[] = {2, (size_t)-1, 8, 2, (size_t)-1, 16, 16, 1, (size_t)-1};
+ return generate_with(events, mixin, 3, amounts_paid, false, range_proof_type, NULL, NULL);
+}
+
+bool gen_bp_txs_valid_2_and_3_and_2_and_4::generate(std::vector& events) const
+{
+ const size_t mixin = 10;
+ const uint64_t amounts_paid[] = {1000, 1000, (uint64_t)-1, 1000, 1000, 1000, (uint64_t)-1, 1000, 1000, (uint64_t)-1, 1000, 1000, 1000, 1000, (uint64_t)-1};
+ const rct::RangeProofType range_proof_type[] = {rct::RangeProofPaddedBulletproof, rct::RangeProofPaddedBulletproof, rct::RangeProofPaddedBulletproof, rct::RangeProofPaddedBulletproof};
+ const size_t bp_sizes[] = {2, (size_t)-1, 3, (size_t)-1, 2, (size_t)-1, 4, (size_t)-1};

If I activated the `check_bp` post_tx test, this test failed which was fixed by
«`diff
— const size_t bp_sizes[] = {2, (size_t)-1, 3, (size_t)-1, 2, (size_t)-1, 4, (size_t)-1};
+ const size_t bp_sizes[] = {2, (size_t)-1, 4, (size_t)-1, 2, (size_t)-1, 4, (size_t)-1};
«`
apparently for the same reason with the `gen_bp_tx_valid_3` case (sizes need to be bumped up to power of 2).

> +
+bool gen_bp_tx_valid_1::generate(std::vector& events) const
+{
+ const size_t mixin = 10;
+ const uint64_t amounts_paid[] = {10000, (uint64_t)-1};
+ const size_t bp_sizes[] = {1, (size_t)-1};
+ const rct::RangeProofType range_proof_type[] = {rct::RangeProofPaddedBulletproof};
+ return generate_with(events, mixin, 1, amounts_paid, true, range_proof_type, NULL, [&](const cryptonote::transaction &tx, size_t tx_idx){ return check_bp(tx, tx_idx, bp_sizes, «gen_bp_tx_valid_1»); });
+}
+
+bool gen_bp_tx_invalid_1_1::generate(std::vector& events) const
+{
+ const size_t mixin = 10;
+ const uint64_t amounts_paid[] = {5000, 5000, (uint64_t)-1};
+ const size_t bp_sizes[] = {1, 1, (size_t)-1};
+ const rct::RangeProofType range_proof_type[] = { rct::RangeProofBulletproof };

I haven’t really dived into the main code yet, so my understanding on the distinction between different types of BPs is still vague, but why is the use of `RangeProofBulletproof` supposed to cause failure here? Also, I see all the valid core tests always using the `RangeProofPaddedBulletproof` type. Shouldn’t there be valid tests for the other 2 types, `RangeProofBulletproof` and `RangeProofMultiOutputBulletproof`?

Добавить комментарий