Page MenuHomePhabricator

[avalanche] Move proof verification to processor
ClosedPublic

Authored by Fabien on Apr 21 2021, 07:52.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABC35e52efad474: [avalanche] Move proof verification to processor
Summary

This is an attempt to fix the processor initialization by adding a
factory method to build a Processor which will check the input arguments
consistency and build a Processor instance accordingly, or return a null
ptr and an error string. The Processor is only built if the arguments
are right. A few additional checks are added for the private keys.

This is based on D9368.

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
processor_factory
Lint
Lint Errors
SeverityLocationCodeMessage
Errorsrc/avalanche/test/processor_tests.cpp:422CPPCHECKcontainerOutOfBounds
Errorsrc/avalanche/test/processor_tests.cpp:423CPPCHECKcontainerOutOfBounds
Errorsrc/avalanche/test/processor_tests.cpp:443CPPCHECKcontainerOutOfBounds
Errorsrc/avalanche/test/processor_tests.cpp:444CPPCHECKcontainerOutOfBounds
Unit
No Test Coverage
Build Status
Buildable 15556
Build 31032: Build Diffbuild-without-wallet · build-clang-tidy · lint-circular-dependencies · build-diff · build-clang · build-debug
Build 31031: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.Apr 21 2021, 07:52

Note to reviewers: the lint errors are false positive from a cppcheck bug introduced in a recent version. There is no such issue if you run arc lint against this patch on Debian.

deadalnix requested changes to this revision.Apr 21 2021, 12:08
deadalnix added a subscriber: deadalnix.

Ok, now this is looking more like something that make sense. There is this encoding/decoding of the session key that needs to be addressed, but beside that, it's all good.

src/avalanche/processor.cpp
265 ↗(On Diff #28240)

Why are you encoding the session key and then decoding it? That makes no sense.

This revision now requires changes to proceed.Apr 21 2021, 12:08

Don't encode/decode the session key

deadalnix requested changes to this revision.Apr 21 2021, 18:26
deadalnix added inline comments.
src/avalanche/processor.cpp
172

You are taking ownership of this key. So that it by value not ref. It leaves the caller with the option to move or copy depending on need.

This revision now requires changes to proceed.Apr 21 2021, 18:26

Pass the session key by value and move it

Tail of the build log:

rpc_psbt.py                                      | ✓ Passed  | 19 s
rpc_rawtransaction.py                            | ✓ Passed  | 38 s
rpc_scantxoutset.py                              | ✓ Passed  | 3 s
rpc_setban.py                                    | ✓ Passed  | 2 s
rpc_signmessage.py                               | ✓ Passed  | 1 s
rpc_signrawtransaction.py                        | ✓ Passed  | 1 s
rpc_txoutproof.py                                | ✓ Passed  | 2 s
rpc_uptime.py                                    | ✓ Passed  | 1 s
rpc_users.py                                     | ✓ Passed  | 5 s
rpc_whitelist.py                                 | ✓ Passed  | 1 s
tool_wallet.py                                   | ✓ Passed  | 4 s
wallet_abandonconflict.py                        | ✓ Passed  | 5 s
wallet_address_types.py                          | ✓ Passed  | 15 s
wallet_avoidreuse.py                             | ✓ Passed  | 3 s
wallet_backup.py                                 | ✓ Passed  | 30 s
wallet_balance.py                                | ✓ Passed  | 16 s
wallet_basic.py                                  | ✓ Passed  | 32 s
wallet_coinbase_category.py                      | ✓ Passed  | 1 s
wallet_create_tx.py                              | ✓ Passed  | 5 s
wallet_createwallet.py                           | ✓ Passed  | 2 s
wallet_createwallet.py --usecli                  | ✓ Passed  | 3 s
wallet_descriptor.py                             | ✓ Passed  | 6 s
wallet_disable.py                                | ✓ Passed  | 1 s
wallet_dump.py                                   | ✓ Passed  | 5 s
wallet_encryption.py                             | ✓ Passed  | 5 s
wallet_groups.py                                 | ✓ Passed  | 52 s
wallet_hd.py                                     | ✓ Passed  | 5 s
wallet_import_rescan.py                          | ✓ Passed  | 5 s
wallet_import_with_label.py                      | ✓ Passed  | 1 s
wallet_importdescriptors.py                      | ✓ Passed  | 5 s
wallet_importmulti.py                            | ✓ Passed  | 3 s
wallet_importprunedfunds.py                      | ✓ Passed  | 2 s
wallet_keypool.py                                | ✓ Passed  | 3 s
wallet_keypool_topup.py                          | ✓ Passed  | 3 s
wallet_labels.py                                 | ✓ Passed  | 2 s
wallet_listreceivedby.py                         | ✓ Passed  | 15 s
wallet_listsinceblock.py                         | ✓ Passed  | 2 s
wallet_listtransactions.py                       | ✓ Passed  | 10 s
wallet_multiwallet.py                            | ✓ Passed  | 14 s
wallet_multiwallet.py --usecli                   | ✓ Passed  | 16 s
wallet_reorgsrestore.py                          | ✓ Passed  | 3 s
wallet_resendwallettransactions.py               | ✓ Passed  | 5 s
wallet_txn_clone.py                              | ✓ Passed  | 1 s
wallet_txn_clone.py --mineblock                  | ✓ Passed  | 3 s
wallet_txn_doublespend.py                        | ✓ Passed  | 2 s
wallet_txn_doublespend.py --mineblock            | ✓ Passed  | 3 s
wallet_watchonly.py                              | ✓ Passed  | 1 s
wallet_watchonly.py --usecli                     | ✓ Passed  | 1 s
wallet_zapwallettxes.py                          | ✓ Passed  | 4 s

ALL                                              | ✓ Passed  | 911 s (accumulated) 
Runtime: 182 s

----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK

ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1

The build failure is an unrelated leveldb spurious failure

This revision is now accepted and ready to land.Apr 22 2021, 11:16
This revision was landed with ongoing or failed builds.Apr 22 2021, 12:01
This revision was automatically updated to reflect the committed changes.