Expired transactions with expiration time later than last block are possible to be included in a new block

[abitmore]

The expiration field of transactions is checked in _apply_transaction() in db_block.cpp (code):

      fc::time_point_sec now = head_block_time();
      FC_ASSERT( trx.expiration <= now + chain_parameters.maximum_time_until_expiration, "",
                 ("trx.expiration",trx.expiration)("now",now)("max_til_exp",chain_parameters.maximum_time_until_expiration));
      FC_ASSERT( now <= trx.expiration, "", ("now",now)("trx.exp",trx.expiration) );

head_block_time() is implemented in db_getter.cpp (code):

time_point_sec database::head_block_time()const
{
   return get( dynamic_global_property_id_type() ).time;
}

The time field is updated in update_global_dynamic_data() in db_update.cpp (code):

void database::update_global_dynamic_data( const signed_block& b )
{
   ...
   modify( _dgp, [&]( dynamic_global_property_object& dgp ){
      ...
      dgp.time = b.timestamp;
      ...

But _apply_transaction() is called before update_global_dynamic_data(next_block) in _apply_block() (code):

   for( const auto& trx : next_block.transactions )
   {
      apply_transaction( trx, skip );
      ++_current_trx_in_block;
   }

   update_global_dynamic_data(next_block);

So theoretically if a transaction's expiration field is later than last block's timestamp, but earlier than the new block's timestamp, it can still be included in the new block and pass the check? Please let me know if I'm wrong.

[abitmore]

I guess it won't happen, since I haven't found an instance from the chain. Perhaps it's avoided by some other code that I've overlooked.

[pmconrad]

So theoretically if a transaction's expiration field is later than last block's timestamp, but earlier than the new block's timestamp, it can still be included in the new block and pass the check?

I think you're right.
Probably hasn't happened in practice because it requires tight timing to create the issue, and why should anyone want to do that?

Not a big issue though, IMO. Modifying apply_block to set the new head_block_time before applying txs is likely to interfere with HF dates, so there is a risk associated with fixing that. Perhaps add a check in _generate_block?

[abitmore]

Won't fix.