Skip to content

Commit

Permalink
Merge pull request #1645 from AntelopeIO/GH-1614-rm-max-non-privilege…
Browse files Browse the repository at this point in the history
…d-inline-action-size

Remove max-nonprivileged-inline-action-size option
  • Loading branch information
heifner authored Sep 15, 2023
2 parents 76a1045 + d8f99d5 commit 5958385
Show file tree
Hide file tree
Showing 16 changed files with 89 additions and 100 deletions.
4 changes: 0 additions & 4 deletions docs/01_nodeos/03_plugins/chain_plugin/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,6 @@ Config Options for eosio::chain_plugin:

--enable-account-queries arg (=0) enable queries to find accounts by
various metadata.
--max-nonprivileged-inline-action-size arg (=4096)
maximum allowed size (in bytes) of an
inline action for a nonprivileged
account
--transaction-retry-max-storage-size-gb arg
Maximum size (in GiB) allowed to be
allocated for the Transaction Retry
Expand Down
13 changes: 0 additions & 13 deletions libraries/chain/apply_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -357,12 +357,6 @@ void apply_context::execute_inline( action&& a ) {
control.check_actor_list( actors );
}

if( !privileged && control.is_speculative_block() ) {
const auto& chain_config = control.get_global_properties().configuration;
EOS_ASSERT( a.data.size() < std::min(chain_config.max_inline_action_size, control.get_max_nonprivileged_inline_action_size()),
inline_action_too_big_nonprivileged,
"inline action too big for nonprivileged account ${account}", ("account", a.account));
}
// No need to check authorization if replaying irreversible blocks or contract is privileged
if( !control.skip_auth_check() && !privileged && !trx_context.is_read_only() ) {
try {
Expand Down Expand Up @@ -417,13 +411,6 @@ void apply_context::execute_context_free_inline( action&& a ) {
EOS_ASSERT( a.authorization.size() == 0, action_validate_exception,
"context-free actions cannot have authorizations" );

if( !privileged && control.is_speculative_block() ) {
const auto& chain_config = control.get_global_properties().configuration;
EOS_ASSERT( a.data.size() < std::min(chain_config.max_inline_action_size, control.get_max_nonprivileged_inline_action_size()),
inline_action_too_big_nonprivileged,
"inline action too big for nonprivileged account ${account}", ("account", a.account));
}

auto inline_receiver = a.account;
_cfa_inline_actions.emplace_back(
schedule_action( std::move(a), inline_receiver, true )
Expand Down
8 changes: 1 addition & 7 deletions libraries/chain/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1258,8 +1258,7 @@ struct controller_impl {
|| (code == contract_blacklist_exception::code_value)
|| (code == action_blacklist_exception::code_value)
|| (code == key_blacklist_exception::code_value)
|| (code == sig_variable_size_limit_exception::code_value)
|| (code == inline_action_too_big_nonprivileged::code_value);
|| (code == sig_variable_size_limit_exception::code_value);
}

bool scheduled_failure_is_subjective( const fc::exception& e ) const {
Expand Down Expand Up @@ -2747,11 +2746,6 @@ subjective_billing& controller::get_mutable_subjective_billing() {
}


uint32_t controller::get_max_nonprivileged_inline_action_size()const
{
return my->conf.max_nonprivileged_inline_action_size;
}

controller::controller( const controller::config& cfg, const chain_id_type& chain_id )
:my( new controller_impl( cfg, *this, protocol_feature_set{}, chain_id ) )
{
Expand Down
1 change: 0 additions & 1 deletion libraries/chain/include/eosio/chain/config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ const static uint32_t default_sig_cpu_bill_pct = 50 * perc
const static uint32_t default_block_cpu_effort_pct = 90 * percent_1; // percentage of block time used for producing block
const static uint16_t default_controller_thread_pool_size = 2;
const static uint32_t default_max_variable_signature_length = 16384u;
const static uint32_t default_max_nonprivileged_inline_action_size = 4 * 1024; // 4 KB
const static uint32_t default_max_action_return_value_size = 256;

const static uint32_t default_max_transaction_finality_status_success_duration_sec = 180;
Expand Down
2 changes: 0 additions & 2 deletions libraries/chain/include/eosio/chain/controller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ namespace eosio { namespace chain {
uint64_t state_guard_size = chain::config::default_state_guard_size;
uint32_t sig_cpu_bill_pct = chain::config::default_sig_cpu_bill_pct;
uint16_t thread_pool_size = chain::config::default_controller_thread_pool_size;
uint32_t max_nonprivileged_inline_action_size = chain::config::default_max_nonprivileged_inline_action_size;
bool read_only = false;
bool force_all_checks = false;
bool disable_replay_opts = false;
Expand Down Expand Up @@ -198,7 +197,6 @@ namespace eosio { namespace chain {
const protocol_feature_manager& get_protocol_feature_manager()const;
const subjective_billing& get_subjective_billing()const;
subjective_billing& get_mutable_subjective_billing();
uint32_t get_max_nonprivileged_inline_action_size()const;

const flat_set<account_name>& get_actor_whitelist() const;
const flat_set<account_name>& get_actor_blacklist() const;
Expand Down
3 changes: 1 addition & 2 deletions libraries/chain/include/eosio/chain/exceptions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,7 @@ namespace eosio { namespace chain {
3050010, "Action attempts to increase RAM usage of account without authorization" )
FC_DECLARE_DERIVED_EXCEPTION( restricted_error_code_exception, action_validate_exception,
3050011, "eosio_assert_code assertion failure uses restricted error code value" )
FC_DECLARE_DERIVED_EXCEPTION( inline_action_too_big_nonprivileged, action_validate_exception,
3050012, "Inline action exceeds maximum size limit for a non-privileged account" )
// Removed 3050012 - inline_action_too_big_nonprivileged, no longer needed
FC_DECLARE_DERIVED_EXCEPTION( action_return_value_exception, action_validate_exception,
3050014, "action return value size too big" )

Expand Down
11 changes: 4 additions & 7 deletions libraries/testing/include/eosio/testing/tester.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ namespace eosio { namespace testing {

virtual ~base_tester() {};

void init(const setup_policy policy = setup_policy::full, db_read_mode read_mode = db_read_mode::HEAD, std::optional<uint32_t> genesis_max_inline_action_size = std::optional<uint32_t>{}, std::optional<uint32_t> config_max_nonprivileged_inline_action_size = std::optional<uint32_t>{});
void init(const setup_policy policy = setup_policy::full, db_read_mode read_mode = db_read_mode::HEAD, std::optional<uint32_t> genesis_max_inline_action_size = std::optional<uint32_t>{});
void init(controller::config config, const snapshot_reader_ptr& snapshot);
void init(controller::config config, const genesis_state& genesis);
void init(controller::config config);
Expand Down Expand Up @@ -395,7 +395,7 @@ namespace eosio { namespace testing {
return genesis;
}

static std::pair<controller::config, genesis_state> default_config(const fc::temp_directory& tempdir, std::optional<uint32_t> genesis_max_inline_action_size = std::optional<uint32_t>{}, std::optional<uint32_t> config_max_nonprivileged_inline_action_size = std::optional<uint32_t>{}) {
static std::pair<controller::config, genesis_state> default_config(const fc::temp_directory& tempdir, std::optional<uint32_t> genesis_max_inline_action_size = std::optional<uint32_t>{}) {
controller::config cfg;
cfg.blocks_dir = tempdir.path() / config::default_blocks_dir_name;
cfg.state_dir = tempdir.path() / config::default_state_dir_name;
Expand All @@ -419,9 +419,6 @@ namespace eosio { namespace testing {
if (genesis_max_inline_action_size) {
gen.initial_configuration.max_inline_action_size = *genesis_max_inline_action_size;
}
if (config_max_nonprivileged_inline_action_size) {
cfg.max_nonprivileged_inline_action_size = *config_max_nonprivileged_inline_action_size;
}
return {cfg, gen};
}

Expand Down Expand Up @@ -452,8 +449,8 @@ namespace eosio { namespace testing {

class tester : public base_tester {
public:
tester(setup_policy policy = setup_policy::full, db_read_mode read_mode = db_read_mode::HEAD, std::optional<uint32_t> genesis_max_inline_action_size = std::optional<uint32_t>{}, std::optional<uint32_t> config_max_nonprivileged_inline_action_size = std::optional<uint32_t>{}) {
init(policy, read_mode, genesis_max_inline_action_size, config_max_nonprivileged_inline_action_size);
tester(setup_policy policy = setup_policy::full, db_read_mode read_mode = db_read_mode::HEAD, std::optional<uint32_t> genesis_max_inline_action_size = std::optional<uint32_t>{}) {
init(policy, read_mode, genesis_max_inline_action_size);
}

tester(controller::config config, const genesis_state& genesis) {
Expand Down
4 changes: 2 additions & 2 deletions libraries/testing/tester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ namespace eosio { namespace testing {
return control->head_block_id() == other.control->head_block_id();
}

void base_tester::init(const setup_policy policy, db_read_mode read_mode, std::optional<uint32_t> genesis_max_inline_action_size, std::optional<uint32_t> config_max_nonprivileged_inline_action_size) {
auto def_conf = default_config(tempdir, genesis_max_inline_action_size, config_max_nonprivileged_inline_action_size);
void base_tester::init(const setup_policy policy, db_read_mode read_mode, std::optional<uint32_t> genesis_max_inline_action_size) {
auto def_conf = default_config(tempdir, genesis_max_inline_action_size);
def_conf.first.read_mode = read_mode;
cfg = def_conf.first;

Expand Down
4 changes: 0 additions & 4 deletions plugins/chain_plugin/chain_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,6 @@ void chain_plugin::set_program_options(options_description& cli, options_descrip
"'none' - EOS VM OC tier-up is completely disabled.\n")
#endif
("enable-account-queries", bpo::value<bool>()->default_value(false), "enable queries to find accounts by various metadata.")
("max-nonprivileged-inline-action-size", bpo::value<uint32_t>()->default_value(config::default_max_nonprivileged_inline_action_size), "maximum allowed size (in bytes) of an inline action for a nonprivileged account")
("transaction-retry-max-storage-size-gb", bpo::value<uint64_t>(),
"Maximum size (in GiB) allowed to be allocated for the Transaction Retry feature. Setting above 0 enables this feature.")
("transaction-retry-interval-sec", bpo::value<uint32_t>()->default_value(20),
Expand Down Expand Up @@ -633,9 +632,6 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) {
if( options.count( "chain-state-db-guard-size-mb" ))
chain_config->state_guard_size = options.at( "chain-state-db-guard-size-mb" ).as<uint64_t>() * 1024 * 1024;

if( options.count( "max-nonprivileged-inline-action-size" ))
chain_config->max_nonprivileged_inline_action_size = options.at( "max-nonprivileged-inline-action-size" ).as<uint32_t>();

if( options.count( "transaction-finality-status-max-storage-size-gb" )) {
const uint64_t max_storage_size = options.at( "transaction-finality-status-max-storage-size-gb" ).as<uint64_t>() * 1024 * 1024 * 1024;
if (max_storage_size > 0) {
Expand Down
4 changes: 0 additions & 4 deletions tests/PerformanceHarness/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1365,8 +1365,6 @@ Finally, the full detail test report for each of the determined max TPS throughp
"_enableAccountQueriesNodeosDefault": 0,
"_enableAccountQueriesNodeosArg": "--enable-account-queries",
"maxNonprivilegedInlineActionSize": null,
"_maxNonprivilegedInlineActionSizeNodeosDefault": 4096,
"_maxNonprivilegedInlineActionSizeNodeosArg": "--max-nonprivileged-inline-action-size",
"transactionRetryMaxStorageSizeGb": null,
"_transactionRetryMaxStorageSizeGbNodeosDefault": null,
"_transactionRetryMaxStorageSizeGbNodeosArg": "--transaction-retry-max-storage-size-gb",
Expand Down Expand Up @@ -2033,8 +2031,6 @@ The Performance Test Basic generates, by default, a report that details results
"_enableAccountQueriesNodeosDefault": 0,
"_enableAccountQueriesNodeosArg": "--enable-account-queries",
"maxNonprivilegedInlineActionSize": null,
"_maxNonprivilegedInlineActionSizeNodeosDefault": 4096,
"_maxNonprivilegedInlineActionSizeNodeosArg": "--max-nonprivileged-inline-action-size",
"transactionRetryMaxStorageSizeGb": null,
"_transactionRetryMaxStorageSizeGbNodeosDefault": null,
"_transactionRetryMaxStorageSizeGbNodeosArg": "--transaction-retry-max-storage-size-gb",
Expand Down
62 changes: 18 additions & 44 deletions unittests/api_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1435,10 +1435,16 @@ BOOST_FIXTURE_TEST_CASE(transaction_tests, validating_tester) { try {
// test send_action_empty
CALL_TEST_FUNCTION(*this, "test_transaction", "send_action_empty", {});

// test send_action_large
BOOST_CHECK_EXCEPTION(CALL_TEST_FUNCTION(*this, "test_transaction", "send_action_large", {}), inline_action_too_big_nonprivileged,
// test send_action_large (512k)
CALL_TEST_FUNCTION( *this, "test_transaction", "send_action_512k", {});

// test send_many_actions_512k (512k)
CALL_TEST_FUNCTION( *this, "test_transaction", "send_many_actions_512k", {});

// test send_action_large (512k + 1)
BOOST_CHECK_EXCEPTION(CALL_TEST_FUNCTION(*this, "test_transaction", "send_action_large", {}), inline_action_too_big,
[](const fc::exception& e) {
return expect_assert_message(e, "inline action too big for nonprivileged account");
return expect_assert_message(e, "inline action too big");
}
);

Expand Down Expand Up @@ -1559,12 +1565,12 @@ BOOST_FIXTURE_TEST_CASE(transaction_tests, validating_tester) { try {
} FC_LOG_AND_RETHROW() }

/*************************************************************************************
* verify subjective limit test case
* verify objective limit test case
*************************************************************************************/
BOOST_AUTO_TEST_CASE(inline_action_subjective_limit) { try {
BOOST_AUTO_TEST_CASE(inline_action_with_over_4k_limit) { try {
const uint32_t _4k = 4 * 1024;
tester chain(setup_policy::full, db_read_mode::HEAD, {_4k + 100}, {_4k + 1});
tester chain2(setup_policy::full, db_read_mode::HEAD, {_4k + 100}, {_4k});
tester chain(setup_policy::full, db_read_mode::HEAD, {_4k + 100});
tester chain2(setup_policy::full, db_read_mode::HEAD, {_4k + 100});
signed_block_ptr block;
for (int n=0; n < 2; ++n) {
block = chain.produce_block();
Expand All @@ -1591,7 +1597,7 @@ BOOST_AUTO_TEST_CASE(inline_action_subjective_limit) { try {
*************************************************************************************/
BOOST_AUTO_TEST_CASE(inline_action_objective_limit) { try {
const uint32_t _4k = 4 * 1024;
tester chain(setup_policy::full, db_read_mode::HEAD, {_4k}, {_4k - 1});
tester chain(setup_policy::full, db_read_mode::HEAD, {_4k});
chain.produce_blocks(2);
chain.create_account( "testapi"_n );
chain.produce_blocks(100);
Expand All @@ -1611,42 +1617,10 @@ BOOST_AUTO_TEST_CASE(inline_action_objective_limit) { try {

} FC_LOG_AND_RETHROW() }

BOOST_AUTO_TEST_CASE(deferred_inline_action_subjective_limit_failure) { try {
const uint32_t _4k = 4 * 1024;
tester chain(setup_policy::full, db_read_mode::HEAD, {_4k + 100}, {_4k});
chain.produce_blocks(2);
chain.create_accounts( {"testapi"_n, "testapi2"_n, "alice"_n} );
chain.set_code( "testapi"_n, test_contracts::test_api_wasm() );
chain.set_code( "testapi2"_n, test_contracts::test_api_wasm() );
chain.produce_block();

transaction_trace_ptr trace;
auto c = chain.control->applied_transaction.connect([&](std::tuple<const transaction_trace_ptr&, const packed_transaction_ptr&> x) {
auto& t = std::get<0>(x);
if (t->scheduled) { trace = t; }
} );
CALL_TEST_FUNCTION(chain, "test_transaction", "send_deferred_transaction_4k_action", {} );
BOOST_CHECK(!trace);
BOOST_CHECK_EXCEPTION(chain.produce_block( fc::seconds(2) ), fc::exception,
[](const fc::exception& e) {
return expect_assert_message(e, "inline action too big for nonprivileged account");
}
);

//check that it populates exception fields
BOOST_REQUIRE(trace);
BOOST_REQUIRE(trace->except);
BOOST_REQUIRE(trace->error_code);

BOOST_REQUIRE_EQUAL(1, trace->action_traces.size());
c.disconnect();

} FC_LOG_AND_RETHROW() }

BOOST_AUTO_TEST_CASE(deferred_inline_action_subjective_limit) { try {
BOOST_AUTO_TEST_CASE(deferred_inline_action_limit) { try {
const uint32_t _4k = 4 * 1024;
tester chain(setup_policy::full, db_read_mode::HEAD, {_4k + 100}, {_4k + 1});
tester chain2(setup_policy::full, db_read_mode::HEAD, {_4k + 100}, {_4k});
tester chain(setup_policy::full, db_read_mode::HEAD, {_4k + 100});
tester chain2(setup_policy::full, db_read_mode::HEAD, {_4k + 100});
signed_block_ptr block;
for (int n=0; n < 2; ++n) {
block = chain.produce_block();
Expand Down Expand Up @@ -1674,7 +1648,7 @@ BOOST_AUTO_TEST_CASE(deferred_inline_action_subjective_limit) { try {

//confirm printed message
BOOST_TEST(!trace->action_traces.empty());
BOOST_TEST(trace->action_traces.back().console == "exec 8");
BOOST_TEST(trace->action_traces.back().console == "action size: 4096");
c.disconnect();

for (int n=0; n < 10; ++n) {
Expand Down
6 changes: 6 additions & 0 deletions unittests/test-contracts/test_api/test_action.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ void test_action::test_dummy_action() {
}
}

void test_action::read_action() {
print("action size: " + std::to_string(action_data_size()));
void* p = malloc(action_data_size());
read_action_data(p, action_data_size());
}

void test_action::read_action_to_0() {
read_action_data( (void *)0, action_data_size() );
}
Expand Down
3 changes: 3 additions & 0 deletions unittests/test-contracts/test_api/test_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ extern "C" {

//test_action
WASM_TEST_HANDLER ( test_action, read_action_normal );
WASM_TEST_HANDLER ( test_action, read_action );
WASM_TEST_HANDLER ( test_action, read_action_to_0 );
WASM_TEST_HANDLER ( test_action, read_action_to_64k );
WASM_TEST_HANDLER_EX( test_action, require_notice );
Expand Down Expand Up @@ -118,6 +119,8 @@ extern "C" {
WASM_TEST_HANDLER ( test_transaction, send_action_empty );
WASM_TEST_HANDLER ( test_transaction, send_action_large );
WASM_TEST_HANDLER ( test_transaction, send_action_4k );
WASM_TEST_HANDLER ( test_transaction, send_action_512k );
WASM_TEST_HANDLER ( test_transaction, send_many_actions_512k );
WASM_TEST_HANDLER ( test_transaction, send_action_recurse );
WASM_TEST_HANDLER ( test_transaction, test_read_transaction );
WASM_TEST_HANDLER ( test_transaction, test_transaction_size );
Expand Down
3 changes: 3 additions & 0 deletions unittests/test-contracts/test_api/test_api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ struct test_print {

struct test_action {
static void read_action_normal();
static void read_action();
static void read_action_to_0();
static void read_action_to_64k();
static void test_dummy_action();
Expand Down Expand Up @@ -202,6 +203,8 @@ struct test_transaction {
static void send_action_max();
static void send_action_large();
static void send_action_4k();
static void send_action_512k();
static void send_many_actions_512k();
static void send_action_recurse();
static void send_action_inline_fail();
static void test_read_transaction();
Expand Down
Binary file modified unittests/test-contracts/test_api/test_api.wasm
Binary file not shown.
Loading

0 comments on commit 5958385

Please sign in to comment.