Merge pull request #5316 from ethereum/sstore_refund_fix

Constantinople SSTORE refund fix
ethereum · Oct 15, 2018 · 8daa6e1 · 8daa6e1
2 parents 1289bc0 + 7dc6ccd
commit 8daa6e1
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 17 deletions.
diff --git a/libethereum/Executive.cpp b/libethereum/Executive.cpp
@@ -534,14 +534,16 @@ bool Executive::go(OnOpFunc const& _onOp)
 
 bool Executive::finalize()
 {
-    // Accumulate refunds for suicides.
     if (m_ext)
+    {
+        // Accumulate refunds for suicides.
         m_ext->sub.refunds += m_ext->evmSchedule().suicideRefundGas * m_ext->sub.suicides.size();
 
-    // SSTORE refunds...
-    // must be done before the miner gets the fees.
-    m_refunded = m_ext ? min((m_t.gas() - m_gas) / 2, m_ext->sub.refunds) : 0;
-    m_gas += m_refunded;
+        // Refunds must be applied before the miner gets the fees.
+        assert(m_ext->sub.refunds >= 0);
+        int64_t maxRefund = (static_cast<int64_t>(m_t.gas()) - static_cast<int64_t>(m_gas)) / 2;
+        m_gas += min(maxRefund, m_ext->sub.refunds);
+    }
 
     if (m_t)
     {

diff --git a/libethereum/Executive.h b/libethereum/Executive.h
@@ -201,7 +201,6 @@ class Executive
     TransactionException m_excepted = TransactionException::None;	///< Details if the VM's execution resulted in an exception.
     int64_t m_baseGasRequired;			///< The base amount of gas requried for executing this transaction.
     u256 m_gas = 0;						///< The gas for EVM code execution. Initial amount before go() execution, final amount after go() execution.
-    u256 m_refunded = 0;				///< The amount of gas refunded.
 
     Transaction m_t;					///< The original transaction. Set by setup().
     LogEntries m_logs;					///< The log entries created by this transaction. Set by finalize().

diff --git a/libevm/ExtVMFace.cpp b/libevm/ExtVMFace.cpp
@@ -84,10 +84,7 @@ evmc_storage_status setStorage(evmc_context* _context, evmc_address const* _addr
         if (originalValue != 0)
         {
             if (currentValue == 0)
-            {
-                assert(env.sub.refunds >= schedule.sstoreRefundGas);
-                env.sub.refunds -= schedule.sstoreRefundGas;
-            }
+                env.sub.refunds -= schedule.sstoreRefundGas;  // Can go negative.
             if (newValue == 0)
                 env.sub.refunds += schedule.sstoreRefundGas;
         }

diff --git a/libevm/ExtVMFace.h b/libevm/ExtVMFace.h
@@ -88,9 +88,9 @@ class owning_bytes_ref: public vector_ref<byte const>
 
 struct SubState
 {
-    std::set<Address> suicides;    ///< Any accounts that have suicided.
-    LogEntries logs;            ///< Any logs.
-    u256 refunds;                ///< Refund counter of SSTORE nonzero->zero.
+    std::set<Address> suicides;  ///< Any accounts that have suicided.
+    LogEntries logs;             ///< Any logs.
+    int64_t refunds = 0;         ///< Refund counter for storage changes.
 
     SubState& operator+=(SubState const& _s)
     {

diff --git a/libevm/LegacyVM.cpp b/libevm/LegacyVM.cpp
@@ -143,10 +143,7 @@ void LegacyVM::updateSSGasEIP1283(u256 const& _currentValue, u256 const& _newVal
             if (originalValue != 0)
             {
                 if (_currentValue == 0)
-                {
-                    assert(m_ext->sub.refunds >= m_schedule->sstoreRefundGas);
-                    m_ext->sub.refunds -= m_schedule->sstoreRefundGas;
-                }
+                    m_ext->sub.refunds -= m_schedule->sstoreRefundGas;  // Can go negative.
                 if (_newValue == 0)
                     m_ext->sub.refunds += m_schedule->sstoreRefundGas;
             }