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

Expected tests for protocol upgrade functionality #6736

Closed
arhag opened this issue Feb 13, 2019 · 1 comment
Closed

Expected tests for protocol upgrade functionality #6736

arhag opened this issue Feb 13, 2019 · 1 comment

Comments

@arhag
Copy link
Contributor

arhag commented Feb 13, 2019

Plan for developing the protocol upgrade functionality

Several changes to the core code base are needed to facilitate protocol upgrade features.

First, it is necessary to build the foundations for protocol feature upgrades before a single (pre-activated) protocol feature of interest can actually be implemented.

The first part of building the foundations has already been completed. PR #6588 and PR #6624 which complete the refactors of block header state and fork database in preparation for the changes needed for protocol features have already been merged into a separate branch protocol-feature-foundations. This branch (which is forked off of a recent develop branch) will host the changes for the protocol feature foundations. In addition to changes from PR #6588 and PR #6624 (and possibly some code changes that may be required to satisfy the requirements on irreversible mode documented in #6704), the main portion of the protocol feature foundations are the changes tracked by issues #6429, #6431, and #6437.

After the protocol feature foundations are completed, the protocol-feature-foundations branch will be forked off to the forced-replay branch which will be kept in sync with develop. The idea will be for the individual protocol features to merged into this branch (and any other PRs assuming replay from genesis is required) as they are developed rather than into develop directly. The protocol feature foundations include large changes that will force a replay from genesis. We do not want to merge these changes into develop, which would commit those changes to the next minor release of eosio release candidate, until we are confident that we will be able to get the desired set of initial protocol features in together with the protocol feature foundations for an RC release.

In particular, it is important to ensure that the protocol feature tracked in issue #6103 is at least included in this initial set prior to a release, since that is the protocol feature that imposes the requirement of forced replay from genesis. In general, it would be ideal to include the protocol features intended for the initial set of protocol features that require a change to ChainBase data structures. The protocol features that require a change to the ChainBase data structures (and thus also a change to the deterministic snapshot specification) are labeled with SNAPSHOT on GitHub; not all protocol feature issues labeled SNAPSHOTS on the eos GitHub will be included in the initial set of protocol features to be released.

See the section below called "Candidates for initial protocol features" for the candidate set of protocol upgrade features to be considered for the initial release and the tests (at a high-level) expected for each.

Testing the protocol feature foundations

The protocol feature foundations will require testing.

The protocol upgrade tracked by #6431 (pre-activation intrinsic) itself acts as a good test of #6429 and especially #6437.

A sufficient automated test of #6437 (and a significant part of #6429) would be to:

  1. Start off with the base protocol and no contracts deployed on the eosio account.
  2. Attempt and fail to deploy a new version of the eosio.bios contract that links to the new intrinsics introduced in Consensus protocol upgrade to enable protocol feature pre-activation #6431.
  3. Activate the protocol feature for Consensus protocol upgrade to enable protocol feature pre-activation #6431.
  4. Deploy the new version of the eosio.bios contract that links to the new intrinsics introduced in Consensus protocol upgrade to enable protocol feature pre-activation #6431.
  5. Test an action that uses the is_feature_active intrinsic.

(The above test is handled in protocol_feature_tests/activate_preactivate_feature from PR #6831.)

Instead of duplicating work to create a fake protocol upgrade feature solely to test #6431 that then needs to somehow be special cased to not actually be supported in production, it is simpler to just use one of the protocol upgrades already planned as the test case of the functionality of #6431 (and by extension more of the functionality of #6429).

A sufficient automated test of #6431 would be to:

  1. Setup the test chain with Dynamic whitelist of intrinsics #6437 activated (requires doing steps 1, 3, then 4 from above) with the protocol features config setup to require the target protocol feature to need pre-activation.
  2. Verify that the old behavior of the target protocol feature is enforced.
  3. Attempt and fail to activate the target protocol feature without pre-activation.
  4. Push an action that pre-activates the target protocol feature.
  5. Verify that creating the next block that avoids activating the target protocol feature is not acceptable to other chains.
  6. Actually create the next block properly by activating the target protocol feature.
  7. Verify that the new behavior of the target protocol feature is enforced.

(All of the above parts of the test are handled in protocol_feature_tests/only_link_to_existing_permission_test and protocol_feature_tests/require_preactivation_test from PRs #6831 and #7006.)

These tests would also be testing the functionality of #6429 (at least the parts of intended to be implemented for the first protocol upgrade release of eosio). However, it is desirable to create additional automated tests to handle some additional scenarios related to protocol upgrade activation. These tests include:

To test that the nodeos process and its plugins do not alter the intended behavior that would already be tested at the libchain level with the automated tests listed above, Python tests are also added:

Candidates for initial protocol features

In addition to the protocol feature foundations, the release of eosio that introduces support for protocol upgrades will come with support for some initial set of protocol features. The candidates for this initial set (not all of the protocol features below will necessarily be included in the first release) are listed below along associated testing requirements:

  1. Disallow linking to non-existing permission (codename: ONLY_LINK_TO_EXISTING_PERMISSION, details at Consensus upgrade to disallow linking to non-existing permission #6333)

    Testing requirements:

    • Check that it is possible to link an action to a non-existing permission prior to ONLY_LINK_TO_EXISTING_PERMISSION activation.
    • Check that it is not possible to link an action to a non-existing permission prior to ONLY_LINK_TO_EXISTING_PERMISSION activation.

(The above test is handled in protocol_feature_tests/only_link_to_existing_permission_test from PR #6831.)

  1. Fix excessive restrictions of eosio::linkauth (codename: FIX_LINKAUTH_RESTRICTION, details at Consensus protocol upgrade to fix excessive restrictions of eosio::linkauth #6672)

    Testing requirements:

    • Check that it is not possible to link actions (of a non-system contract) with any of the unintentionally restricted names (updateauth, deleteauth, linkauth, unlinkauth, canceldelay) prior to FIX_LINKAUTH_RESTRICTION activation.
    • Check that it is possible to link actions (of a non-system contract) with any of the unintentionally restricted names (updateauth, deleteauth, linkauth, unlinkauth, canceldelay) prior to FIX_LINKAUTH_RESTRICTION activation.

(The above test is handled in protocol_feature_tests/fix_linkauth_restriction from PR #7025.)

  1. Disallow proposing an empty producer schedule (codename: DISALLOW_EMPTY_PRODUCER_SCHEDULE, details at Consensus protocol upgrade to disallow proposing an empty producer schedule #6458)

    Testing requirements:

    • Check that it is possible to propose an empty producer schedule prior to DISALLOW_EMPTY_PRODUCER_SCHEDULE activation.
    • Check that it is not possible to propose an empty producer schedule prior to DISALLOW_EMPTY_PRODUCER_SCHEDULE activation.

(The above test is handled in protocol_feature_tests/disallow_empty_producer_schedule_test from PR #7026.)

  1. Restrict authorization checking when sending actions to self (codename: RESTRICT_ACTION_TO_SELF, details at Consensus protocol upgrade to restrict authorization checking when sending actions to self #6705)

    Testing requirements:

    • Prior to RESTRICT_ACTION_TO_SELF activation, push an action authorized by alice@active to a contract which sends an inline action to itself with an authorization by alice@active. This should be successful.
    • After RESTRICT_ACTION_TO_SELF activation, check that it is not possible for a contract deployed on an account that is not inline in the permission tree of any other account to:
      • send an inline action to itself in which the action is authorized by an account other than the contract account;
      • or, send a deferred transaction containing an action to itself in which the action is authorized by an account other than the contract account.
      • The above two cases should be handled by a contract executing in the context of both a regular action and an action notification.

(The above test is handled in protocol_feature_tests/restrict_action_to_self_test from PR #7088.)

  1. Fix problems associated with replacing deferred transaction (codename: REPLACE_DEFERRED, details at Consensus upgrade to fix problems associated with replacing deferred transactions #6103)

    Testing requirements:

    • Prior to REPLACE_DEFERRED activation:
      • With the subjective mitigations in place, push two actions that try to each send a deferred transaction from the same contract with same sender_id and verify the second one is rejected.
      • With the subjective mitigations disabled, repeat pushing the prior two actions and verify that the second replaced the first but kept the old transaction ID and did not return the RAM appropriately.
    • Activate REPLACE_DEFERRED and immediately after verify that the RAM usage of the account that lost RAM due to earlier actions has been corrected.
    • After REPLACE_DEFERRED activation, and subjective mitigations again in place, repeat pushing the two actions again, but this time verify that the second properly replaces the first with a new transaction ID and with the RAM usage properly accounted for.

(The above test is handled in protocol_feature_tests/replace_deferred_test from PR #6997.)

  1. Avoid transaction ID collision of deferred transactions (codename: NO_DUPLICATE_DEFERRED_ID, details at Consensus upgrade to avoid transaction ID collision of deferred transactions #6115)

    Testing requirements:

    • Prior to NO_DUPLICATE_DEFERRED_ID activation:
      • Creating a deferred transaction with non-empty transaction extensions is allowed. However, retiring that deferred transaction should fail with a subjective error.
    • After NO_DUPLICATE_DEFERRED_ID activation:
      • Creating a deferred transaction with an invalid transaction extension is not allowed.
      • After creating a deferred transaction with no transaction extension, it should be stored in the generated_transaction_multi_index with the appropriate generation context transaction extension.
      • Verify that a contract can create a deferred transaction with an appropriate generation context transaction extension.

(The above test is handled in protocol_feature_tests/no_duplicate_deferred_id_test from PR #7072.)

  1. Modify restrictions on RAM billing (codename: RAM_RESTRICTIONS, details at Consensus upgrade to modify the restrictions on RAM billing #6105)

    Testing requirements:

    • Prior to RAM_RESTRICTIONS activation:
      • Setup a contract that enables storing some data in a table record with RAM paid by alice and has an action to migrate that record to another table in such a way that continues to bill the same payer but reduces the RAM usage. Provide a way to also trigger that migration operation through a notification rather than a direct action. Furthermore, the contract should have a mechanism (which can be triggered either by direct action or through notification) to schedule a deferred transaction that is billed to an arbitrarily specified account.
      • Verify that calling that migration action for a table record paid by alice fails if the action does not have the authorization of alice. (Test case 1)
      • Verify that calling that migration action for a table record paid by alice succeeds if the action does have the authorization of alice.
      • Verify that calling the action to schedule the deferred transaction paid by alice succeeds if the action is authorized by alice. (Test case 2)
      • Verify that calling the action to schedule the deferred transaction paid by alice fails if the action is not authorized by alice. (Test case 3)
    • Prior to RAM_RESTRICTIONS activation and with the subjective mitigation of v1.2.3 not enforced:
      • Push an action authorized by alice to contract A which notifies contract B which attempts to bill alice. This should be successful. (Test case 4)
    • After RAM_RESTRICTIONS activation:
      • Verify that test case 1 now succeeds.
      • Verify that test case 2 still succeeds.
      • Verify that test case 3 still fails.
      • Verify that test case 4 now fails.
      • Verify that an action (that is not authorized by alice) which increases the RAM usage of alice still fails.

(The above test is handled in protocol_feature_tests/ram_restrictions_test from PR #7131.)

  1. Only bill CPU and network bandwidth to first authorizer of a transaction (codename: ONLY_BILL_FIRST_AUTHORIZER, details at Consensus upgrade to only bill CPU and network bandwidth to first authorizer of a transaction #6332)

    Testing requirements:

    • Prior to ONLY_BILL_FIRST_AUTHORIZER activation:
      • Push a transaction authorized by two accounts. Verify that the network (and CPU) usage of both accounts increases by the same amount.
    • After ONLY_BILL_FIRST_AUTHORIZER activation:
      • Push a transaction authorized by two accounts. Verify that the network and CPU usage of the first authorizing account increases. Verify that the network and CPU usage of the other account has not changed.

(The above test is handled in protocol_feature_tests/only_bill_to_first_authorizer from PR #7089.)

  1. Forward setcode action to WebAssembly code deployed on eosio account (codename: FORWARD_SETCODE, details at Consensus protocol upgrade to forward setcode actions to the contract on the eosio account #6988)

    Testing requirements:

    • Prior to FORWARD_SETCODE activation:
      • With a contract to reject all actions (including eosio::setcode but not if it's setting code on the eosio account) deployed on eosio, ensure that it is still possible to set code on some other arbitrary account.
    • After FORWARD_SETCODE activation:
      • With a contract to reject all actions (including eosio::setcode but not if it's setting code on the eosio account) deployed on eosio, ensure that it is not possible to set code on some other arbitrary account but it is possible to replace the code on the eosio account with the bios contract.
      • Ensure it is not possible to set the code on the eosio account to a contract that will reject all actions with no exception.

(The above test is handled in protocol_feature_tests/forward_setcode_test from PR #7109.)

  1. Allows contracts to determine which account is the sender of an inline action (codename: GET_SENDER, details at Consensus protocol upgrade to get the sending account of an inline action #7028)

    Testing requirements:

    • Prior to GET_SENDER activation:
      • Ensure a contract using the get_sender intrinsic cannot be deployed.
    • After GET_SENDER activation:
      • get_sender called from a top-level action should return 0.
      • get_sender called from an inline action should return the receiver of the action that sent the inline action.
      • get_sender called from a notified action execution should return the receiver of the action that first called the require_recipient to notify the contract.

(The above test is handled in protocol_feature_tests/get_sender_test from PR #7111.)

@arhag
Copy link
Contributor Author

arhag commented Apr 19, 2019

All tests in above issue were implemented in various PRs. See the parenthetical notes below each protocol feature in the above list for the relevant PRs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant