Require Payer Authorization on Msig Propose

contracts/README.md

@@ -17,7 +17,7 @@ The collection of system contracts consists of the following individual contract
    + Allocation of CPU/NET resources based on core tokens in which the core tokens are either staked for an indefinite allocation of some fraction of available CPU/NET resources, or they are paid as a fee in exchange for a time-limited allocation of CPU/NET resources via ROA.
    + An automated market maker enabling a market for RAM resources which allows users to buy or sell available RAM allocations.
    + An auction for bidding for premium account names.
-* [multisig contract](contracts/sysio.msig/include/sysio.msig/sysio.msig.hpp): A contract that enables proposing transactions on the blockchain, collecting authorization approvals for many accounts, and then executing the actions within the transaction after authorization requirements of the transaction have been reached. (Note: this contract must be deployed to a privileged account.)
+* [multisig contract](contracts/sysio.msig/include/sysio.msig/sysio.msig.hpp): A contract that enables proposing transactions on the blockchain, collecting authorization approvals for many accounts, and then executing the actions within the transaction after authorization requirements of the transaction have been reached. `propose` must carry `{proposer, sysio.payer}` as its first authorization, so the proposer pays its CPU and NET. (Note: this contract must be deployed to a privileged account.)
 * [wrap contract](contracts/sysio.wrap/include/sysio.wrap/sysio.wrap.hpp): A contract that wraps around any transaction and allows for executing its actions without needing to satisfy the authorization requirements of the transaction. If used, the permissions of the account hosting this contract should be configured to only allow highly trusted parties (e.g. the operators of the blockchain) to have the ability to execute its actions. (Note: this contract must be deployed to a privileged account.)
 
 ## Depot flow tests

contracts/sysio.msig/sysio.msig.cpp

@@ -6,6 +6,12 @@
 
 namespace sysio {
 
+namespace {
+/// Marks its actor as the payer of an action's CPU and NET when it is the action's first
+/// authorization (see `action::payer()` on the host).
+constexpr name payer_permission = "sysio.payer"_n;
+}
+
 transaction_header get_trx_header(const char* ptr, size_t sz);
 bool trx_is_authorized(const std::vector<permission_level>& approvals, const std::vector<char>& packed_trx);
 
@@ -120,6 +126,8 @@ void multisig::propose( name proposer,
                         ignore<transaction> trx )
 {
    require_auth( proposer );
+   // The proposer, not msig, pays the CPU and NET of propose, whose data is the proposed transaction.
+   require_auth( permission_level{ proposer, payer_permission } );
    auto& ds = get_datastream();
 
    const char* trx_pos = ds.pos();

contracts/sysio.msig/sysio.msig.hpp

@@ -32,6 +32,9 @@ class [[sysio::contract("sysio.msig")]] multisig : public contract {
     * saved in the proposals table and the `requested` permission levels to the
     * approvals table (for the `proposer` context). Storage changes are billed to `proposer`.
     *
+    * The action must carry `{proposer, sysio.payer}` as its first authorization, so `proposer`
+    * pays its CPU and NET instead of this contract.
+    *
     * @param proposer - The account proposing a transaction
     * @param proposal_name - The name of the proposal (should be unique for proposer)
     * @param requested - Permission levels expected to approve the proposal

contracts/tests/sysio.msig_tests.cpp

@@ -3,6 +3,7 @@
 #include <sysio/chain/abi_serializer.hpp>
 #include <sysio/chain/global_property_object.hpp>
 #include <sysio/chain/kv_table_objects.hpp>
+#include <sysio/chain/resource_limits_private.hpp>
 #include <sysio/chain/wast_to_wasm.hpp>
 
 #include <fc/variant_object.hpp>
@@ -74,26 +75,86 @@ class sysio_msig_tester : public tester {
       return asset( result, symbol(CORE_SYM) );
    }
 
+   /// `{account, sysio.payer}, {account, active}`: `account` authorizes the action and pays its CPU and NET.
+   static vector<permission_level> payer_auths( account_name account ) {
+      return { { account, config::sysio_payer_name }, { account, config::active_name } };
+   }
+
+   /// `propose` must be paid by its signer; the other msig actions bill the contract.
+   static bool requires_payer( const action_name& name ) {
+      return name == "propose"_n;
+   }
+
    transaction_trace_ptr push_action( const account_name& signer, const action_name& name, const variant_object& data, bool auth = true ) {
-      vector<account_name> accounts;
+      vector<permission_level> auths;
       if( auth )
-         accounts.push_back( signer );
-      auto trace = base_tester::push_action( "sysio.msig"_n, name, accounts, data );
+         auths = requires_payer( name ) ? payer_auths( signer )
+                                        : vector<permission_level>{ { signer, config::active_name } };
+      auto trace = base_tester::push_action( "sysio.msig"_n, name, auths, data );
       produce_block();
       BOOST_REQUIRE_EQUAL( true, chain_has_transaction(trace->id) );
       return trace;
+   }
 
-      /*
-         string action_type_name = abi_ser.get_action_type(name);
+   /// CPU and NET usage accumulators of `account`.
+   struct resource_usage {
+      resource_limits::usage_accumulator cpu;
+      resource_limits::usage_accumulator net;
+   };
+
+   resource_usage get_resource_usage( account_name account ) const {
+      const auto& usage = control->db().get<resource_limits::resource_object, resource_limits::by_owner>( account );
+      return { usage.cpu_usage, usage.net_usage };
+   }
+
+   /// Pushes `trx` with its CPU measured rather than the tester's fixed bill, and checks that the whole
+   /// transaction's CPU and NET went to `payer` while every account in `unbilled` was left untouched.
+   /// The pending block is not produced, so the expected usage is the payer's accumulator plus this
+   /// transaction at the pending slot.
+   transaction_trace_ptr push_and_check_billed_to( signed_transaction& trx, account_name payer,
+                                                   const vector<account_name>& unbilled ) {
+      const auto payer_before = get_resource_usage( payer );
+      vector<resource_usage> unbilled_before;
+      for( const auto& account : unbilled )
+         unbilled_before.push_back( get_resource_usage( account ) );
+
+      auto trace = base_tester::push_transaction( trx, fc::time_point::maximum(), measured_cpu );
+
+      const auto& cfg = control->db().get<resource_limits::resource_limits_config_object>();
+      const auto slot = block_timestamp_type( control->pending_block_time() ).slot;
+      auto expected = payer_before;
+      expected.cpu.add( trace->total_cpu_usage_us, slot, cfg.account_cpu_usage_average_window );
+      expected.net.add( trace->net_usage, slot, cfg.account_net_usage_average_window );
+      check_usage_equal( expected, get_resource_usage( payer ) );
+
+      for( size_t i = 0; i < unbilled.size(); ++i )
+         check_usage_equal( unbilled_before[i], get_resource_usage( unbilled[i] ) );
+      return trace;
+   }
 
-         action act;
-         act.account = "sysio.msig"_n;
-         act.name = name;
-         act.data = abi_ser.variant_to_binary( action_type_name, data, fixture_abi_yield_function() );
-         //std::cout << "test:\n" << fc::to_hex(act.data.data(), act.data.size()) << " size = " << act.data.size() << std::endl;
+   static void check_usage_equal( const resource_usage& expected, const resource_usage& actual ) {
+      BOOST_CHECK_EQUAL( expected.cpu.last_ordinal, actual.cpu.last_ordinal );
+      BOOST_CHECK_EQUAL( expected.cpu.value_ex,     actual.cpu.value_ex );
+      BOOST_CHECK_EQUAL( expected.cpu.consumed,     actual.cpu.consumed );
+      BOOST_CHECK_EQUAL( expected.net.last_ordinal, actual.net.last_ordinal );
+      BOOST_CHECK_EQUAL( expected.net.value_ex,     actual.net.value_ex );
+      BOOST_CHECK_EQUAL( expected.net.consumed,     actual.net.consumed );
+   }
 
-         return base_tester::push_action( std::move(act), auth ? uint64_t(signer) : 0 );
-      */
+   /// `billed_cpu_time_us` of 0 makes the chain measure the transaction's CPU.
+   static constexpr uint32_t measured_cpu = 0;
+
+   /// A one-action transaction on `code`, signed with the active key of every non-payer authorizer.
+   signed_transaction create_signed_trx( account_name code, action_name act, const vector<permission_level>& auths,
+                                         const variant_object& data ) {
+      signed_transaction trx;
+      trx.actions.emplace_back( get_action( code, act, auths, data ) );
+      set_transaction_headers( trx );
+      for( const auto& auth : auths ) {
+         if( auth.permission != config::sysio_payer_name )
+            trx.sign( get_private_key( auth.actor, auth.permission.to_string() ), control->get_chain_id() );
+      }
+      return trx;
    }
 
    transaction reqauth( account_name from, const vector<permission_level>& auths, uint32_t delay_sec = 0 );
@@ -185,6 +246,39 @@ BOOST_FIXTURE_TEST_CASE( propose_approve_execute, sysio_msig_tester ) try {
                         } );
 } FC_LOG_AND_RETHROW()
 
+// `propose` bills the proposer: it fails without `{proposer, sysio.payer}`, and when another account is the payer.
+BOOST_FIXTURE_TEST_CASE( propose_requires_payer_auth, sysio_msig_tester ) try {
+   auto trx = reqauth( "alice"_n, {permission_level{"alice"_n, config::active_name}} );
+   const auto propose_data = mvo()
+      ("proposer",      "alice")
+      ("proposal_name", "first")
+      ("trx",           trx)
+      ("requested",     vector<permission_level>{{ "alice"_n, config::active_name }});
+
+   BOOST_REQUIRE_EXCEPTION( base_tester::push_action( "sysio.msig"_n, "propose"_n, "alice"_n, propose_data ),
+                            missing_auth_exception,
+                            fc_exception_message_is("missing authority of alice/sysio.payer") );
+
+   vector<permission_level> bob_pays = payer_auths( "bob"_n );
+   bob_pays.push_back( { "alice"_n, config::active_name } );
+   BOOST_REQUIRE_EXCEPTION( base_tester::push_action( "sysio.msig"_n, "propose"_n, bob_pays, propose_data ),
+                            missing_auth_exception,
+                            fc_exception_message_is("missing authority of alice/sysio.payer") );
+
+   base_tester::push_action( "sysio.msig"_n, "propose"_n, payer_auths( "alice"_n ), propose_data );
+} FC_LOG_AND_RETHROW()
+
+// The proposer pays the CPU and NET of `propose`; msig and the proposal's requested approver are not billed.
+BOOST_FIXTURE_TEST_CASE( propose_billed_to_proposer, sysio_msig_tester ) try {
+   auto proposed = reqauth( "carol"_n, {permission_level{"carol"_n, config::active_name}} );
+   auto propose_trx = create_signed_trx( "sysio.msig"_n, "propose"_n, payer_auths( "alice"_n ), mvo()
+                                            ("proposer",      "alice")
+                                            ("proposal_name", "first")
+                                            ("trx",           proposed)
+                                            ("requested", vector<permission_level>{{ "carol"_n, config::active_name }}) );
+   push_and_check_billed_to( propose_trx, "alice"_n, { "sysio.msig"_n, "carol"_n } );
+} FC_LOG_AND_RETHROW()
+
 
 // A proposal carrying a non-zero `delay_sec` executes once its delay has elapsed, and not
 // before. Nothing else in this suite proposes with a delay, so without this case the whole
@@ -1007,7 +1101,7 @@ BOOST_FIXTURE_TEST_CASE( sendinline, sysio_msig_tester ) try {
 
    transaction trx = reqauth( "alice"_n, {permission_level{"alice"_n, "perm"_n}} );
 
-   base_tester::push_action( "sysio.msig"_n, "propose"_n, "bob"_n, mvo()
+   base_tester::push_action( "sysio.msig"_n, "propose"_n, payer_auths( "bob"_n ), mvo()
                               ("proposer", "bob")
                               ("proposal_name", "first")
                               ("trx", trx)

contracts/tests/sysio.system_tester.hpp

@@ -357,6 +357,19 @@ class sysio_system_tester : public TESTER {
       msig_abi_ser.set_abi(msig_abi, fixture_abi_yield_function());
    }
 
+   /// Pushes a `sysio.msig` action signed by `signer`. `propose` carries `{signer, sysio.payer}` first, as
+   /// msig requires, so `signer` pays; the other msig actions bill the contract.
+   action_result push_action_msig( const account_name& signer, const action_name& name, const variant_object& data ) {
+      action act;
+      act.account = "sysio.msig"_n;
+      act.name = name;
+      act.data = msig_abi_ser.variant_to_binary( msig_abi_ser.get_action_type(name), data, fixture_abi_yield_function() );
+
+      if( name == "propose"_n )
+         return base_tester::push_action( std::move(act), signer.to_uint64_t() );
+      return base_tester::push_contract_paid_action( std::move(act), signer.to_uint64_t() );
+   }
+
    /// Compatibility spelling for activate_producers(). Producer registration itself now requires
    /// the ACTIVE OPERATOR_TYPE_PRODUCER rows that activate_producers installs first.
    ///

contracts/tests/sysio.system_tests.cpp

@@ -248,17 +248,6 @@ BOOST_FIXTURE_TEST_CASE(producers_upgrade_system_contract, sysio_system_tester)
 
    produce_blocks();
 
-   //helper function
-   auto push_action_msig = [&]( const account_name& signer, const action_name &name, const variant_object &data, bool auth = true ) -> action_result {
-         string action_type_name = msig_abi_ser.get_action_type(name);
-
-         action act;
-         act.account = "sysio.msig"_n;
-         act.name = name;
-         act.data = msig_abi_ser.variant_to_binary( action_type_name, data, fixture_abi_yield_function() );
-
-         return base_tester::push_contract_paid_action( std::move(act), (auth ? signer : signer == "bob111111111"_n ? "alice1111111"_n : "bob111111111"_n).to_uint64_t() );
-   };
    // test begins
    vector<permission_level> prod_perms;
    for ( auto& x : producer_names ) {
@@ -380,17 +369,6 @@ BOOST_FIXTURE_TEST_CASE( setparams, sysio_system_tester ) try {
 
    add_roa_policy(NODE_DADDY, "alice1111111"_n, "32.0000 SYS", "32.0000 SYS", "500.0000 SYS", 0, 0);
 
-   //helper function
-   auto push_action_msig = [&]( const account_name& signer, const action_name &name, const variant_object &data, bool auth = true ) -> action_result {
-         string action_type_name = msig_abi_ser.get_action_type(name);
-
-         action act;
-         act.account = "sysio.msig"_n;
-         act.name = name;
-         act.data = msig_abi_ser.variant_to_binary( action_type_name, data, fixture_abi_yield_function() );
-
-         return base_tester::push_contract_paid_action( std::move(act), (auth ? signer : signer == "bob111111111"_n ? "alice1111111"_n : "bob111111111"_n).to_uint64_t() );
-   };
 
    // test begins
    vector<permission_level> prod_perms;
@@ -476,17 +454,6 @@ BOOST_FIXTURE_TEST_CASE( wasmcfg, sysio_system_tester ) try {
 
    add_roa_policy(NODE_DADDY, "alice1111111"_n, "32.0000 SYS", "32.0000 SYS", "500.0000 SYS", 0, 0);
 
-   //helper function
-   auto push_action_msig = [&]( const account_name& signer, const action_name &name, const variant_object &data, bool auth = true ) -> action_result {
-         string action_type_name = msig_abi_ser.get_action_type(name);
-
-         action act;
-         act.account = "sysio.msig"_n;
-         act.name = name;
-         act.data = msig_abi_ser.variant_to_binary( action_type_name, data, fixture_abi_yield_function() );
-
-         return base_tester::push_contract_paid_action( std::move(act), (auth ? signer : signer == "bob111111111"_n ? "alice1111111"_n : "bob111111111"_n).to_uint64_t() );
-   };
 
    // test begins
    vector<permission_level> prod_perms;

docs/contract-upgrade-order.md

@@ -49,7 +49,9 @@ other account in the set**.
 **2. A large set does not fit in one transaction.** The default
 `max_transaction_net_usage` is **524,288** bytes (`config::default_max_block_net_usage
 / 2`), and `sysio.msig::propose` receives the complete inner transaction before it
-chunks storage, so proposing through msig does not dodge that input NET. The
+chunks storage, so proposing through msig does not dodge that input NET. `propose`
+requires `{proposer, sysio.payer}`, so that NET is billed to the proposer, whose ROA
+policy must cover it. The
 contracts on `advance`'s path are not small (sizes of the builds in this tree):
 
 | | code | abi | total |

docs/roa-overview.md

@@ -236,6 +236,9 @@ later quota sync by passing `-1, -1` for a `sysio.`-prefixed owner. The prefix a
 nothing — `activateroa` deliberately creates
 `sysio.acct` at **zero** CPU and NET, a pure RAM bucket for account creation.
 
+`sysio.msig::propose` is the exception: its data is a caller-supplied transaction, so it requires
+`{proposer, sysio.payer}` first and the proposer pays its CPU and NET.
+
 > **What `-1` means here.** It is a **sentinel** meaning "no limit," not a quantity. The chain
 > tests for it explicitly (`is_unlimited_cpu` returns `cpu_weight == -1`) and substitutes a large
 > finite number where arithmetic is needed. Seeing `-1` in a `get_account` response is not an