Removed Deferred Transaction Support from System Contract

[ericpassmore]

System Contract

Remove need_deferred_trx from system contract. Along with updates to delegate_bandwidth.cpp and name_bidding.cpp

eos-system-contracts/contracts/eosio.system/src/delegate_bandwidth.cpp

Lines 324 to 335 in 717537d

	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 );
	}

eos-system-contracts/contracts/eosio.system/src/name_bidding.cpp

Lines 52 to 60 in 717537d

	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 );

[ericpassmore]

Wrap Documentation

Need advice on how to update the Wrap Documentation. Is it still valid to have 0 values for ref_block and ref_block_prefix in the unified transaction proposed to the msig contract?

eos-system-contracts/docs/04_guides/07_how-to-use-eosio.wrap.md

Lines 175 to 205 in 717537d

	Next, the action JSONs of the previously generated transactions will be used to construct a unified transaction which will eventually be proposed with the eosio.msig contract. A good way to get started is to make a copy of the generated_newaccount_trx.json file (call the copied file create_wrap_account_trx.json) and edit the first three fields so it looks something like the following:
	```sh
	cat create_wrap_account_trx.json
	```
	```json
	{
	"expiration": "2018-07-06T12:00:00",
	"ref_block_num": 0,
	"ref_block_prefix": 0,
	"max_net_usage_words": 0,
	"max_cpu_usage_ms": 0,
	"delay_sec": 0,
	"context_free_actions": [],
	"actions": [{
	"account": "eosio",
	"name": "newaccount",
	"authorization": [{
	"actor": "eosio",
	"permission": "active"
	}
	],
	"data": "0000000000ea305500004d1a03ea30550100000000010000000000ea305500000000a8ed32320100000100000000010000000000ea305500000000a8ed3232010000"
	}
	],
	"transaction_extensions": [],
	"signatures": [],
	"context_free_data": []
	}
	```
	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.

[arhag]

@ericpassmore: Yes, ref_block_num and ref_block_prefix fields can remain zero.

The wrap contract does not care about what is in the transaction header other than the expiration. In fact, we may consider the interface for the exec action that takes a transaction to be misleading to the user. It is simply taking a list of actions to send inline.

It does enforce the expiration has not yet been reached, but there is really no point for us to add that check into exec. It also enforces that context_free_actions are empty to not mislead the user into thinking those will run. And yet, it doesn't enforce the value of other fields like ref_block_num, ref_block_prefix, max_net_usage_words, max_cpu_usage_ms, delay_sec even though they have particular meanings. It also does not validate that transaction_extensions are empty even though in theory those extensions could radically change the behavior of regular input transactions in the future.

The multisig contract is in a similar situation. It does enforce that contract_free_actions are empty, that the expiration has not yet been reached (which makes more sense for the multisig to enforce since there is a delay between proposal and execution unlike with the wrap contract), and it does have special rules regarding delay_sec which does change its behavior. But the other fields (ref_block_num, ref_block_prefix, max_net_usage_words, max_cpu_usage_ms, and transaction_extensions) are not validated or used in any way.

Ideally, the multisig contract's propose action interface would also be changed to just take a list of actions, an expiration, and maybe a delay_sec. (Though I also believe that delay_sec should not be provided during transaction proposal time and should instead be delayed until exec which provides more flexibility. See: AntelopeIO/reference-contracts#25.)

Anyway, as the code exists right now, I think it is best that we just continue to document that all those unvalidated fields be either 0 or empty (depending on what is appropriate for the type).

[ericpassmore]

Tests for name bidding are failing. Trying to figure out how all of these is intended to work.

 // start bids
   bidname( "bob",  "prefa", core_sym::from_string("1.0003") );
   BOOST_REQUIRE_EQUAL( core_sym::from_string( "9998.9997" ), get_balance("bob") );
   bidname( "bob",  "prefb", core_sym::from_string("1.0000") );
   bidname( "bob",  "prefc", core_sym::from_string("1.0000") );
   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);
      // as of Leap v5 no longer support deferred trx, bob's balance only changes when winning a bid
      // previously deferred trx would debit balance then refund when canceled
      const asset bob_balance_in_auction = get_balance("bob") ;
      BOOST_REQUIRE_EQUAL( core_sym::from_string( "10000.0000" ), get_balance("alice") );
      BOOST_REQUIRE_EQUAL( success(),
                           bidname( "alice", "prefb", core_sym::from_string("1.1001") ) );

      // This fails: bob's balance should show refund of 1.0000 bid on prefb
      BOOST_REQUIRE_EQUAL( bob_balance_in_auction + core_sym::from_string("1.0000"), get_balance("bob") );
      // alice balance less bid
      BOOST_REQUIRE_EQUAL( core_sym::from_string("10000.0000") - core_sym::from_string("1.1001") , get_balance("alice") );
      // This fails: eosio.names balance should be higher by alice's successful bid
      BOOST_REQUIRE_EQUAL( initial_names_balance + core_sym::from_string("1.1001") , get_balance("eosio.names"_n) );
   }

[ericpassmore]

Solved it with help. Needed to explicitly refund transactions. Use bidrefund action for resource refunds and refund action for bids

// 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( success(), push_action( "carol1111111"_n, "refund"_n, mvo()("owner", "carol1111111") ) );