Collateral asset fee accumulator for BSIPs 74 and 87

libraries/chain/asset_evaluator.cpp

@@ -59,7 +59,7 @@ namespace detail {
    void check_asset_claim_fees_hardfork_87_74_collatfee(const fc::time_point_sec& block_time, const asset_claim_fees_operation& op)
    {
       // HF_REMOVABLE: Following hardfork check should be removable after hardfork date passes:
-      FC_ASSERT( !op.extensions.claim_from_asset_id.valid() ||
+      FC_ASSERT( !op.extensions.value.claim_from_asset_id.valid() ||
                  block_time >= HARDFORK_CORE_BSIP_87_74_COLLATFEE_TIME,
                  "Collateral-denominated fees are not yet active and therefore cannot be claimed." );
    }
@@ -940,20 +940,30 @@ void_result asset_publish_feeds_evaluator::do_apply(const asset_publish_feed_ope
  * The container asset is either explicitly named in the extensions, or else assumed as the same
  * asset as the amount_to_claim asset. Evaluation fails if either (a) operation issuer is not
  * the same as the container_asset issuer, or (b) container_asset has no fee bucket for
- * amount_to_claim asset.
+ * amount_to_claim asset, or (c) accumulated fees are insufficient to cover amount claimed.
  */
 void_result asset_claim_fees_evaluator::do_evaluate( const asset_claim_fees_operation& o )
 { try {
-   database& d = db();
+   const database& d = db();
 
    detail::check_asset_claim_fees_hardfork_87_74_collatfee(d.head_block_time(), o); // HF_REMOVABLE
 
-   const asset_object & container_asset = o.extensions.claim_from_asset_id.valid() ?
-      (*o.extensions.claim_from_asset_id)(d) : o.amount_to_claim.asset_id(d);
-   FC_ASSERT( container_asset.issuer == o.issuer, "Asset fees may only be claimed by the issuer" );
-   FC_ASSERT( container_asset.can_accumulate_fee(d,o.amount_to_claim),
+   container_asset = o.extensions.value.claim_from_asset_id.valid() ?
+      &(*o.extensions.value.claim_from_asset_id)(d) : &o.amount_to_claim.asset_id(d);
+
+   FC_ASSERT( container_asset->issuer == o.issuer, "Asset fees may only be claimed by the issuer" );
+   FC_ASSERT( container_asset->can_accumulate_fee(d,o.amount_to_claim),
               "Asset ${a} (${id}) is not backed by asset (${fid}) and does not hold it as fees.",
-              ("a",container_asset.symbol)("id",container_asset.id)("fid",o.amount_to_claim.asset_id) );
+              ("a",container_asset->symbol)("id",container_asset->id)("fid",o.amount_to_claim.asset_id) );
+
+   container_ddo = &container_asset->dynamic_asset_data_id(d);
+
+   FC_ASSERT( o.amount_to_claim.amount <= ((container_asset->get_id() == o.amount_to_claim.asset_id) ?
+                                           container_ddo->accumulated_fees :
+                                           container_ddo->accumulated_collateral_fees),
+              "Attempt to claim more fees than have accumulated within asset ${a} (${id})",
+              ("a",container_asset->symbol)("id",container_asset->id)("ddo",*container_ddo) );
+
    return void_result();
 } FC_CAPTURE_AND_RETHROW( (o) ) }
 
@@ -965,19 +975,12 @@ void_result asset_claim_fees_evaluator::do_apply( const asset_claim_fees_operati
 { try {
    database& d = db();
 
-   const asset_object & c = o.extensions.claim_from_asset_id.valid() ?
-      (*o.extensions.claim_from_asset_id)(d) : o.amount_to_claim.asset_id(d);
-   const asset_dynamic_data_object& ddo = c.dynamic_asset_data_id(d);
-   const asset_object & a = o.amount_to_claim.asset_id(d);
-
-   if ( c.get_id() == a.get_id() ) {
-      FC_ASSERT( o.amount_to_claim.amount <= ddo.accumulated_fees, "Attempt to claim more fees than have accumulated", ("ddo",ddo) );
-      d.modify( ddo, [&]( asset_dynamic_data_object& _addo  ) {
+   if ( container_asset->get_id() == o.amount_to_claim.asset_id ) {
+      d.modify( *container_ddo, [&o]( asset_dynamic_data_object& _addo  ) {
          _addo.accumulated_fees -= o.amount_to_claim.amount;
       });
    } else {
-      FC_ASSERT( o.amount_to_claim.amount <= ddo.accumulated_collateral_fees, "Attempt to claim more fees than have accumulated", ("ddo",ddo) );
-      d.modify( ddo, [&]( asset_dynamic_data_object& _addo  ) {
+      d.modify( *container_ddo, [&o]( asset_dynamic_data_object& _addo  ) {
          _addo.accumulated_collateral_fees -= o.amount_to_claim.amount;
       });
    }

libraries/chain/asset_object.cpp

@@ -178,7 +178,7 @@ string asset_object::amount_to_string(share_type amount) const
 }
 
 FC_REFLECT_DERIVED_NO_TYPENAME( graphene::chain::asset_dynamic_data_object, (graphene::db::object),
-                    (current_supply)(confidential_supply)(accumulated_fees)(fee_pool) )
+                    (current_supply)(confidential_supply)(accumulated_fees)(accumulated_collateral_fees)(fee_pool) )
 
 FC_REFLECT_DERIVED_NO_TYPENAME( graphene::chain::asset_bitasset_data_object, (graphene::db::object),
                     (asset_id)

libraries/chain/include/graphene/chain/asset_evaluator.hpp

@@ -166,6 +166,9 @@ namespace graphene { namespace chain {
 
          void_result do_evaluate( const asset_claim_fees_operation& o );
          void_result do_apply( const asset_claim_fees_operation& o );
+
+         const asset_object* container_asset = nullptr;
+         const asset_dynamic_data_object* container_ddo = nullptr;
    };
 
    class asset_claim_pool_evaluator : public evaluator<asset_claim_pool_evaluator>

libraries/chain/include/graphene/chain/asset_object.hpp

@@ -184,6 +184,8 @@ namespace graphene { namespace chain {
          template<class DB>
          void accumulate_fee(DB& db, const asset& fee) const
          {
+            if (fee.amount == 0) return;
+            FC_ASSERT( fee.amount >= 0, "Fee amount must be non-negative." );
             const auto& dyn_data = dynamic_asset_data_id(db);
             if (fee.asset_id == get_id()) { // fee same as asset
                db.modify( dyn_data, [&fee]( asset_dynamic_data_object& obj ){
@@ -223,11 +225,6 @@ namespace graphene { namespace chain {
          /// The tunable options for BitAssets are stored in this field.
          bitasset_options options;
 
-         /// Check collateral-denominated fees:
-         template<class DB>
-         bool collateral_fees_are_zero(const DB& db) const
-         { return asset_id(db).dynamic_asset_data_id(db).accumulated_collateral_fees > 0; }
-
          /// Feeds published for this asset. If issuer is not committee, the keys in this map are the feed publishing
          /// accounts; otherwise, the feed publishers are the currently active committee_members and witnesses and this map
          /// should be treated as an implementation detail. The timestamp on each feed is the time it was published.

libraries/protocol/asset_ops.cpp

@@ -251,6 +251,8 @@ void asset_options::validate()const
 void asset_claim_fees_operation::validate()const {
    FC_ASSERT( fee.amount >= 0 );
    FC_ASSERT( amount_to_claim.amount > 0 );
+   if( extensions.value.claim_from_asset_id.valid() )
+     FC_ASSERT( *extensions.value.claim_from_asset_id != amount_to_claim.asset_id );
 }
 
 void asset_claim_pool_operation::validate()const {

libraries/protocol/include/graphene/protocol/asset_ops.hpp

@@ -451,15 +451,16 @@ namespace graphene { namespace protocol {
          /// Which asset to claim fees from. This is needed, e.g., to claim collateral-
          /// denominated fees from a collateral-backed smart asset. If unset, assumed to be same
          /// asset as amount_to_claim is denominated in, such as would be the case when claiming
-         /// market fees.
+         /// market fees. If set, validation requires it to be a different asset_id than
+         /// amount_to_claim (else there would exist two ways to form the same request).
          fc::optional<asset_id_type> claim_from_asset_id;
       };
 
       asset           fee;
-      account_id_type issuer; /// must match issuer of asset from which we claim fees
+      account_id_type issuer; ///< must match issuer of asset from which we claim fees
       asset           amount_to_claim;
 
-      additional_options_type extensions;
+      extension<additional_options_type> extensions;
 
       account_id_type fee_payer()const { return issuer; }
       void            validate()const;