Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove max-nonprivileged-inline-action-size option #1645

Merged
merged 5 commits into from
Sep 15, 2023
Merged

Remove max-nonprivileged-inline-action-size option #1645

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4057c4b
GH-1614 Remove subjective max_nonprivileged_inline_action_size limit
heifner Sep 15, 2023
509c11f
GH-1614 Fix tests for removal of subjective max_nonprivileged_inline_…
heifner Sep 15, 2023
1a7e37a
GH-1614 Remove max-nonprivileged-inline-action-size from docs
heifner Sep 15, 2023
3e3f8d0
GH-1614 remove unused config_max_nonprivileged_inline_action_size option
heifner Sep 15, 2023
d8f99d5
GH-1614 Cleanup inline action tests
heifner Sep 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions libraries/chain/apply_context.cpp
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
9 changes: 3 additions & 6 deletions libraries/testing/include/eosio/testing/tester.hpp
View file
Open in desktop
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 @@ -453,7 +450,7 @@ 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>{}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should you remove the parameter as well, or maybe change the name to unused.
std::optional<uint32_t> config_max_nonprivileged_inline_action_size = std::optional<uint32_t>{}

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, not sure how my search missed that.

init(policy, read_mode, genesis_max_inline_action_size, config_max_nonprivileged_inline_action_size);
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
View file
Open in desktop
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
View file
Open in desktop
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
46 changes: 10 additions & 36 deletions unittests/api_tests.cpp
View file
Open in desktop
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 @@ -1611,38 +1617,6 @@ 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 {
const uint32_t _4k = 4 * 1024;
tester chain(setup_policy::full, db_read_mode::HEAD, {_4k + 100}, {_4k + 1});
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Binary file not shown.
61 changes: 51 additions & 10 deletions unittests/test-contracts/test_api/test_transaction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,15 @@ void test_transaction::send_action_empty() {
}

/**
* cause failure due to a large action payload
* cause failure due to a large action payload, larger than max_inline_action_size of 512K
*/
void test_transaction::send_action_large() {
using namespace eosio;
static char large_message[8 * 1024];
test_action_action<"testapi"_n.value, WASM_TEST_ACTION( "test_action", "read_action_normal" )> test_action;
copy_data( large_message, 8*1024, test_action.data );
test_action_action<"testapi"_n.value, WASM_TEST_ACTION( "test_action", "read_action" )> test_action;
test_action.data.resize(512*1024+1);

std::vector<permission_level> permissions = { {"testapi"_n, "active"_n} };
action act( permissions, name{"testapi"}, name{WASM_TEST_ACTION("test_action", "read_action_normal")}, test_action );

action act( permissions, name{"testapi"}, name{WASM_TEST_ACTION("test_action", "read_action")}, test_action );
act.send();
eosio_assert( false, "send_message_large() should've thrown an error" );
}
Expand All @@ -104,16 +102,59 @@ void test_transaction::send_action_large() {
*/
void test_transaction::send_action_4k() {
using namespace eosio;
static char large_message[4 * 1024];
test_action_action<"testapi"_n.value, WASM_TEST_ACTION( "test_action", "test_action_ordinal4" )> test_action;
copy_data( large_message, 4*1024, test_action.data );
test_action_action<"testapi"_n.value, WASM_TEST_ACTION( "test_action", "read_action" )> test_action;
test_action.data.resize(4*1024);

std::vector<permission_level> permissions = { {"testapi"_n, "active"_n} };
action act( permissions, name{"testapi"}, name{WASM_TEST_ACTION("test_action", "read_action")}, test_action );

act.send();
}

/**
* send an inline action that is 512K (limit is < 512K)
* the limit includes the size of the action
*/
void test_transaction::send_action_512k() {
using namespace eosio;
test_action_action<"testapi"_n.value, WASM_TEST_ACTION( "test_action", "read_action" )> test_action;

test_action.data.resize(1);
std::vector<permission_level> permissions = { {"testapi"_n, "active"_n} };
action act( permissions, name{"testapi"}, name{WASM_TEST_ACTION("test_action", "test_action_ordinal4")}, test_action );
action temp_act( permissions, name{"testapi"}, name{WASM_TEST_ACTION("test_action", "read_action")}, test_action );

size_t action_size = pack_size(temp_act);
test_action.data.resize(512*1024-action_size-2); // check is < 512K

// send at limit (512K - 1)
action act( permissions, name{"testapi"}, name{WASM_TEST_ACTION("test_action", "read_action")}, test_action );

if (pack_size(act) != 512*1024-1) {
std::string err = "send_action_512k action size is: " + std::to_string(action_size) + " not 512K-1";
eosio_assert(false, err.c_str());
}

act.send();
}

/**
* send many inline actions that are 512K (limit is < 512K)
* the limit includes the size of the action
*/
void test_transaction::send_many_actions_512k() {
using namespace eosio;
test_action_action<"testapi"_n.value, WASM_TEST_ACTION( "test_transaction", "send_action_512k" )> test_action;

test_action.data.resize(1);
std::vector<permission_level> permissions = { {"testapi"_n, "active"_n} };
action act( permissions, name{"testapi"}, name{WASM_TEST_ACTION("test_transaction", "send_action_512k")}, test_action );

// 65 * 512K > wasm memory limit, which is ok because each gets their own wasm instantiation
for (size_t i = 0; i < 65; ++i) {
act.send();
}
}

/**
* cause failure due recursive loop
*/
Expand Down
Loading