Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Add multi-sig recovery tests. stat-291 #1466

Merged
merged 2 commits into from
Feb 26, 2018

Conversation

cijujohn
Copy link
Contributor

Add multi-sig recovery test.
Enable misc_tests.

@@ -14,13 +14,26 @@ BOOST_FIXTURE_TEST_CASE( missing_sigs, tester ) { try {
produce_block();

BOOST_REQUIRE_THROW( push_reqauth( N(alice), {permission_level{N(alice), config::active_name}}, {} ), tx_missing_sigs );
auto trace = push_reqauth(N(alice), {permission_level{N(alice), config::active_name}}, { get_private_key(N(alice), "active") } );
auto trace = push_reqauth(N(alice), "owner");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while more concise, this does change the semantics by signing with the "owner" key/permission instead of the active.

We should probably just leave the exploded version that was in place before


produce_block();
produce_block();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix whitespace

@@ -179,6 +179,16 @@ namespace eosio { namespace testing {
return push_transaction( trx );
}

transaction_trace base_tester::push_reqauth(account_name from, string role, bool multi_sig) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be an "up-port" of the method call that used to only exist for recovery tests.

In the context of recovery tests using the "owner" permission level makes sense however, in most other cases that seems to be a mis-use as the concept of "owner" is that it is very rarely used.

It seems this was done to make the missing_multi_sigs tests case more concise but as that could be done with the "Active" permission level and the push_reqauth that already existed with a few more keys strokes maybe we should keep this "owner" bomb in the recover tester?

@wanderingbort
Copy link
Contributor

@cj-oci please address any of the comments in a new PR if you agree with them.

@wanderingbort wanderingbort merged commit 356a8c2 into EOSIO:master Feb 26, 2018
@cijujohn cijujohn deleted the accountRecoveryTest-dawn291 branch May 7, 2018 13:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants