Skip to content

Commit

Permalink
Merge pull request #29 from AntelopeIO/ehp/GH-28-rm-deferred-from-system
Browse files Browse the repository at this point in the history
Remove Deferred Transactions from System Contract
  • Loading branch information
ericpassmore authored Sep 22, 2023
2 parents 76197b4 + 65783cb commit a199ba0
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 36 deletions.
19 changes: 0 additions & 19 deletions contracts/eosio.system/src/delegate_bandwidth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,6 @@ namespace eosiosystem {
//create/update/delete refund
auto net_balance = stake_net_delta;
auto cpu_balance = stake_cpu_delta;
bool need_deferred_trx = false;


// net and cpu are same sign by assertions in delegatebw and undelegatebw
// redundant assertion also at start of changebw to protect against misuse of changebw
Expand Down Expand Up @@ -296,9 +294,6 @@ namespace eosiosystem {

if ( req->is_empty() ) {
refunds_tbl.erase( req );
need_deferred_trx = false;
} else {
need_deferred_trx = true;
}
} else if ( net_balance.amount < 0 || cpu_balance.amount < 0 ) { //need to create refund
refunds_tbl.emplace( from, [&]( refund_request& r ) {
Expand All @@ -317,23 +312,9 @@ namespace eosiosystem {
}
r.request_time = current_time_point();
});
need_deferred_trx = true;
} // else stake increase requested with no existing row in refunds_tbl -> nothing to do with refunds_tbl
} /// end if is_delegating_to_self || is_undelegating

if ( need_deferred_trx ) {
eosio::transaction out;
out.actions.emplace_back( permission_level{from, active_permission},
get_self(), "refund"_n,
from
);
out.delay_sec = refund_delay_sec;
eosio::cancel_deferred( from.value ); // TODO: Remove this line when replacing deferred transactions is fixed
out.send( from.value, from, true );
} else {
eosio::cancel_deferred( from.value );
}

auto transfer_amount = net_balance + cpu_balance;
if ( 0 < transfer_amount.amount ) {
token::transfer_action transfer_act{ token_account, { {source_stake_from, active_permission} } };
Expand Down
10 changes: 0 additions & 10 deletions contracts/eosio.system/src/name_bidding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,6 @@ namespace eosiosystem {
});
}

eosio::transaction t;
t.actions.emplace_back( permission_level{current->high_bidder, active_permission},
get_self(), "bidrefund"_n,
std::make_tuple( current->high_bidder, newname )
);
t.delay_sec = 0;
uint128_t deferred_id = (uint128_t(newname.value) << 64) | current->high_bidder.value;
eosio::cancel_deferred( deferred_id );
t.send( deferred_id, bidder );

bids.modify( current, bidder, [&]( auto& b ) {
b.high_bidder = bidder;
b.high_bid = bid.amount;
Expand Down
2 changes: 1 addition & 1 deletion docs/04_guides/01_upgrading-the-eosio.system-contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ diff original_system_contract.abi new_system_contract.abi
```json
584,592d583
< },{
< "name": "deferred_trx_id",
< "name": "trx_id",
< "type": "uint32"
< },{
< "name": "last_unstake_time",
Expand Down
8 changes: 4 additions & 4 deletions docs/04_guides/07_how-to-use-eosio.wrap.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ cat create_wrap_account_trx.json
}
```

The `ref_block_num` and `ref_block_prefix` values were set to 0. The proposed transaction does not need to have a valid TaPoS reference block because it will be reset anyway when scheduled as a deferred transaction during the `eosio.msig::exec` action. The `expiration` field, which was the only other field that was changed, will also be reset when the proposed transaction is scheduled as a deferred transaction during `eosio.msig::exec`. However, this field actually does matter during the propose-approve-exec lifecycle of the proposed transaction. If the present time passes the time in the `expiration` field of the proposed transaction, it will not be possible to execute the proposed transaction even if all necessary approvals are gathered. Therefore, it is important to set the expiration time to some point well enough in the future to give all necessary approvers enough time to review and approve the proposed transaction, but it is otherwise arbitrary. Generally, for reviewing/validation purposes it is important that all potential approvers of the transaction (i.e. the block producers) choose the exact same `expiration` time so that there is not any discrepancy in bytes of the serialized transaction if it was to later be included in payload data of some other action.
The `ref_block_num` and `ref_block_prefix` values were set to 0. The proposed transaction does not need to have a valid TaPoS reference block because it will be reset anyway when scheduled during the `eosio.msig::exec` action. The `expiration` field, which was the only other field that was changed, will also be reset when the proposed transaction is scheduled during `eosio.msig::exec`. However, this field actually does matter during the propose-approve-exec lifecycle of the proposed transaction. If the present time passes the time in the `expiration` field of the proposed transaction, it will not be possible to execute the proposed transaction even if all necessary approvals are gathered. Therefore, it is important to set the expiration time to some point well enough in the future to give all necessary approvers enough time to review and approve the proposed transaction, but it is otherwise arbitrary. Generally, for reviewing/validation purposes it is important that all potential approvers of the transaction (i.e. the block producers) choose the exact same `expiration` time so that there is not any discrepancy in bytes of the serialized transaction if it was to later be included in payload data of some other action.

Then, all but the first action JSON object of generated_account_creation_trx.json should be appended to the `actions` array of create_wrap_account_trx.json, and then the single action JSON object of generated_setpriv_trx.json should be appended to the `actions` array of create_wrap_account_trx.json. The final result is a create_wrap_account_trx.json file that looks like the following:
```sh
Expand Down Expand Up @@ -387,7 +387,7 @@ warning: transaction executed locally, but may not be confirmed by the network y

### 1.1.4 Execute the transaction to create the eosio.wrap account

When the necessary approvals are collected (in this example, with 21 block producers, at least 15 of their approvals were required), anyone can push the `eosio.msig::exec` action which executes the approved transaction. It makes a lot of sense for the lead block producer who proposed the transaction to also execute it (this will incur another temporary RAM cost for the deferred transaction that is generated by the eosio.msig contract).
When the necessary approvals are collected (in this example, with 21 block producers, at least 15 of their approvals were required), anyone can push the `eosio.msig::exec` action which executes the approved transaction. It makes a lot of sense for the lead block producer who proposed the transaction to also execute it (this will incur another temporary RAM cost for the that is generated by the eosio.msig contract).

```sh
cleos multisig exec blkproducera createwrap blkproducera
Expand Down Expand Up @@ -605,7 +605,7 @@ warning: transaction executed locally, but may not be confirmed by the network y

### 1.2.4 Execute the transaction to create the eosio.wrap account

When the necessary approvals are collected (in this example, with 21 block producers, at least 15 of their approvals were required), anyone can push the `eosio.msig::exec` action which executes the approved transaction. It makes a lot of sense for the lead block producer who proposed the transaction to also execute it (this will incur another temporary RAM cost for the deferred transaction that is generated by the eosio.msig contract).
When the necessary approvals are collected (in this example, with 21 block producers, at least 15 of their approvals were required), anyone can push the `eosio.msig::exec` action which executes the approved transaction. It makes a lot of sense for the lead block producer who proposed the transaction to also execute it (this will incur another temporary RAM cost that is generated by the eosio.msig contract).

```sh
cleos multisig exec blkproducera deploywrap blkproducera
Expand Down Expand Up @@ -929,7 +929,7 @@ warning: transaction executed locally, but may not be confirmed by the network y

### 2.1.4 Execute the transaction to change the owner permission of an account

When the necessary approvals are collected (in this example, with 21 block producers, at least 15 of their approvals were required), anyone can push the `eosio.msig::exec` action which executes the approved transaction. It makes a lot of sense for the lead block producer who proposed the transaction to also execute it (this will incur another temporary RAM cost for the deferred transaction that is generated by the eosio.msig contract).
When the necessary approvals are collected (in this example, with 21 block producers, at least 15 of their approvals were required), anyone can push the `eosio.msig::exec` action which executes the approved transaction. It makes a lot of sense for the lead block producer who proposed the transaction to also execute it (this will incur another temporary RAM cost that is generated by the eosio.msig contract).

```sh
cleos multisig exec blkproducera updatealice blkproducera
Expand Down
25 changes: 23 additions & 2 deletions tests/eosio.system_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,18 @@ BOOST_FIXTURE_TEST_CASE( stake_unstake, eosio_system_tester ) try {

produce_block( fc::hours(3*24-1) );
produce_blocks(1);
// testing balance still the same
BOOST_REQUIRE_EQUAL( core_sym::from_string("700.0000"), get_balance( "alice1111111" ) );
BOOST_REQUIRE_EQUAL( init_eosio_stake_balance + core_sym::from_string("300.0000"), get_balance( "eosio.stake"_n ) );
//after 3 days funds should be released
// call refund expected to fail too early
BOOST_REQUIRE_EQUAL( wasm_assert_msg("refund is not available yet"),
push_action( "alice1111111"_n, "refund"_n, mvo()("owner", "alice1111111") ) );

// after 1 hour refund ready
produce_block( fc::hours(1) );
produce_blocks(1);
// now we can do the refund
BOOST_REQUIRE_EQUAL( success(), push_action( "alice1111111"_n, "refund"_n, mvo()("owner", "alice1111111") ) );
BOOST_REQUIRE_EQUAL( core_sym::from_string("1000.0000"), get_balance( "alice1111111" ) );
BOOST_REQUIRE_EQUAL( init_eosio_stake_balance, get_balance( "eosio.stake"_n ) );

Expand Down Expand Up @@ -235,6 +242,7 @@ BOOST_FIXTURE_TEST_CASE( stake_unstake, eosio_system_tester ) try {

REQUIRE_MATCHING_OBJECT( voter( "alice1111111", core_sym::from_string("0.0000") ), get_voter_info( "alice1111111" ) );
produce_blocks(1);
BOOST_REQUIRE_EQUAL( success(), push_action( "alice1111111"_n, "refund"_n, mvo()("owner", "alice1111111") ) );
BOOST_REQUIRE_EQUAL( core_sym::from_string("1000.0000"), get_balance( "alice1111111" ) );
} FC_LOG_AND_RETHROW()

Expand Down Expand Up @@ -278,6 +286,7 @@ BOOST_FIXTURE_TEST_CASE( stake_unstake_with_transfer, eosio_system_tester ) try
produce_block( fc::hours(1) );
produce_blocks(1);

BOOST_REQUIRE_EQUAL( success(), push_action( "alice1111111"_n, "refund"_n, mvo()("owner", "alice1111111") ) );
BOOST_REQUIRE_EQUAL( core_sym::from_string("1300.0000"), get_balance( "alice1111111" ) );

//stake should be equal to what was staked in constructor, voting power should be 0
Expand Down Expand Up @@ -342,6 +351,7 @@ BOOST_FIXTURE_TEST_CASE( stake_while_pending_refund, eosio_system_tester ) try {
produce_block( fc::hours(1) );
produce_blocks(1);

BOOST_REQUIRE_EQUAL( success(), push_action( "alice1111111"_n, "refund"_n, mvo()("owner", "alice1111111") ) );
BOOST_REQUIRE_EQUAL( core_sym::from_string("1300.0000"), get_balance( "alice1111111" ) );

//stake should be equal to what was staked in constructor, voting power should be 0
Expand Down Expand Up @@ -604,6 +614,7 @@ BOOST_FIXTURE_TEST_CASE( adding_stake_partial_unstake, eosio_system_tester ) try
BOOST_REQUIRE_EQUAL( core_sym::from_string("550.0000"), get_balance( "alice1111111" ) );
produce_block( fc::days(1) );
produce_blocks(1);
BOOST_REQUIRE_EQUAL( success(), push_action( "alice1111111"_n, "refund"_n, mvo()("owner", "alice1111111") ) );
BOOST_REQUIRE_EQUAL( core_sym::from_string("850.0000"), get_balance( "alice1111111" ) );

} FC_LOG_AND_RETHROW()
Expand Down Expand Up @@ -1078,6 +1089,9 @@ BOOST_FIXTURE_TEST_CASE( vote_for_producer, eosio_system_tester, * boost::unit_t
//carol1111111 should receive funds in 3 days
produce_block( fc::days(3) );
produce_block();

// do a bid refund for carol
BOOST_REQUIRE_EQUAL( success(), push_action( "carol1111111"_n, "refund"_n, mvo()("owner", "carol1111111") ) );
BOOST_REQUIRE_EQUAL( core_sym::from_string("3000.0000"), get_balance( "carol1111111" ) );

} FC_LOG_AND_RETHROW()
Expand Down Expand Up @@ -3296,7 +3310,7 @@ BOOST_FIXTURE_TEST_CASE( multiple_namebids, eosio_system_tester ) try {
BOOST_REQUIRE_EQUAL( success(), regproducer( "producer"_n ) );

produce_block();
// stake but but not enough to go live
// stake but not enough to go live
stake_with_transfer( config::system_account_name, "bob"_n, core_sym::from_string( "35000000.0000" ), core_sym::from_string( "35000000.0000" ) );
stake_with_transfer( config::system_account_name, "carl"_n, core_sym::from_string( "35000000.0000" ), core_sym::from_string( "35000000.0000" ) );
BOOST_REQUIRE_EQUAL( success(), vote( "bob"_n, { "producer"_n } ) );
Expand All @@ -3320,11 +3334,14 @@ BOOST_FIXTURE_TEST_CASE( multiple_namebids, eosio_system_tester ) try {
BOOST_REQUIRE_EQUAL( core_sym::from_string( "9996.9997" ), get_balance("bob") );
BOOST_REQUIRE_EQUAL( core_sym::from_string( "10000.0000" ), get_balance("alice") );


// alice outbids bob on prefb
{
const asset initial_names_balance = get_balance("eosio.names"_n);
BOOST_REQUIRE_EQUAL( success(),
bidname( "alice", "prefb", core_sym::from_string("1.1001") ) );
// refund bob's failed bid on prefb
BOOST_REQUIRE_EQUAL( success(), push_action( "bob"_n, "bidrefund"_n, mvo()("bidder","bob")("newname", "prefb") ) );
BOOST_REQUIRE_EQUAL( core_sym::from_string( "9997.9997" ), get_balance("bob") );
BOOST_REQUIRE_EQUAL( core_sym::from_string( "9998.8999" ), get_balance("alice") );
BOOST_REQUIRE_EQUAL( initial_names_balance + core_sym::from_string("0.1001"), get_balance("eosio.names"_n) );
Expand All @@ -3336,6 +3353,8 @@ BOOST_FIXTURE_TEST_CASE( multiple_namebids, eosio_system_tester ) try {
BOOST_REQUIRE_EQUAL( core_sym::from_string( "10000.0000" ), get_balance("david") );
BOOST_REQUIRE_EQUAL( success(),
bidname( "david", "prefd", core_sym::from_string("1.9900") ) );
// refund carls's failed bid on prefd
BOOST_REQUIRE_EQUAL( success(), push_action( "carl"_n, "bidrefund"_n, mvo()("bidder","carl")("newname", "prefd") ) );
BOOST_REQUIRE_EQUAL( core_sym::from_string( "9999.0000" ), get_balance("carl") );
BOOST_REQUIRE_EQUAL( core_sym::from_string( "9998.0100" ), get_balance("david") );
}
Expand Down Expand Up @@ -4745,6 +4764,8 @@ BOOST_FIXTURE_TEST_CASE( ramfee_namebid_to_rex, eosio_system_tester ) try {
BOOST_REQUIRE_EQUAL( success(), bidname( carol, "rndmbid"_n, core_sym::from_string("23.7000") ) );
BOOST_REQUIRE_EQUAL( core_sym::from_string("23.7000"), get_balance( "eosio.names"_n ) );
BOOST_REQUIRE_EQUAL( success(), bidname( alice, "rndmbid"_n, core_sym::from_string("29.3500") ) );
// refund carol, the losing bid
BOOST_REQUIRE_EQUAL( success(), push_action( carol, "bidrefund"_n, mvo()("bidder","carolaccount")("newname", "rndmbid") ) );
BOOST_REQUIRE_EQUAL( core_sym::from_string("29.3500"), get_balance( "eosio.names"_n ));

produce_block( fc::hours(24) );
Expand Down

0 comments on commit a199ba0

Please sign in to comment.