EVMC gas refunds

libaleth-interpreter/VM.cpp

@@ -39,6 +39,7 @@ evmc_result execute(evmc_vm* _instance, const evmc_host_interface* _host,
         output = vm->exec(_host, _context, _rev, _msg, _code, _codeSize);
         result.status_code = EVMC_SUCCESS;
         result.gas_left = vm->m_io_gas;
+        result.gas_refunded = vm->m_gas_refunded;
     }
     catch (dev::eth::RevertInstruction& ex)
     {
@@ -365,7 +366,13 @@ void VM::interpretCases()
                 }
             }
 
-            m_host->selfdestruct(m_context, &m_message->destination, &destination);
+            std::cerr << "SELFD " << fromEvmC(m_message->destination);
+            if (m_host->selfdestruct(m_context, &m_message->destination, &destination))
+            {
+                m_gas_refunded += 15000;
+                std::cerr << " REF";
+            }
+            std::cerr << "\n";
             m_bounce = nullptr;
         }
         BREAK
@@ -1376,21 +1383,66 @@ void VM::interpretCases()
             auto const status =
                 m_host->set_storage(m_context, &m_message->destination, &key, &value);
 
-            switch(status)
+            const auto netStorageCost = (m_rev == EVMC_CONSTANTINOPLE || m_rev >= EVMC_ISTANBUL);
+            if (!netStorageCost)
             {
-            case EVMC_STORAGE_ADDED:
-                m_runGas = VMSchedule::sstoreSetGas;
-                break;
-            case EVMC_STORAGE_MODIFIED:
-            case EVMC_STORAGE_DELETED:
-                m_runGas = VMSchedule::sstoreResetGas;
-                break;
-            case EVMC_STORAGE_UNCHANGED:
-            case EVMC_STORAGE_MODIFIED_AGAIN:
-                m_runGas = (m_rev == EVMC_CONSTANTINOPLE || m_rev >= EVMC_ISTANBUL) ?
-                               (*m_metrics)[OP_SLOAD].gas_cost :
-                               VMSchedule::sstoreResetGas;
-                break;
+                switch (status)
+                {
+                case EVMC_STORAGE_ADDED:
+                    m_runGas = VMSchedule::sstoreSetGas;
+                    break;
+                case EVMC_STORAGE_DELETED:
+                    m_runGas = VMSchedule::sstoreResetGas;
+                    m_gas_refunded += VMSchedule::sstoreRefundGas;
+                    break;
+                default:
+                    m_runGas = VMSchedule::sstoreResetGas;
+                    break;
+                }
+            }
+            else
+            {
+                const auto sstoreUnchangedGas = (*m_metrics)[OP_SLOAD].gas_cost;
+                switch (status)
+                {
+                case EVMC_STORAGE_UNCHANGED:
+                    m_runGas = sstoreUnchangedGas;
+                    break;
+                case EVMC_STORAGE_ADDED:
+                    m_runGas = VMSchedule::sstoreSetGas;
+                    break;
+                case EVMC_STORAGE_MODIFIED:
+                    m_runGas = VMSchedule::sstoreResetGas;
+                    break;
+                case EVMC_STORAGE_DELETED:
+                    m_runGas = VMSchedule::sstoreResetGas;
+                    m_gas_refunded += VMSchedule::sstoreRefundGas;
+                    break;
+                case EVMC_DIRTY_ADDED_TO_DELETED:
+                    m_runGas = sstoreUnchangedGas;
+                    m_gas_refunded = VMSchedule::sstoreSetGas - sstoreUnchangedGas;
+                    break;
+                case EVMC_DIRTY_DELETED_REVERTED:
+                    m_runGas = sstoreUnchangedGas;
+                    m_gas_refunded += VMSchedule::sstoreResetGas - sstoreUnchangedGas -
+                                      VMSchedule::sstoreRefundGas;
+                    break;
+                case EVMC_DIRTY_DELETED_TO_ADDED:
+                    m_runGas = sstoreUnchangedGas;
+                    m_gas_refunded += -VMSchedule::sstoreRefundGas;
+                    break;
+                case EVMC_DIRTY_MODIFIED_TO_DELETED:
+                    m_runGas = sstoreUnchangedGas;
+                    m_gas_refunded += VMSchedule::sstoreRefundGas;
+                    break;
+                case EVMC_DIRTY_MODIFIED_REVERTED:
+                    m_runGas = sstoreUnchangedGas;
+                    m_gas_refunded += VMSchedule::sstoreResetGas - sstoreUnchangedGas;
+                    break;
+                case EVMC_DIRTY_MODIFIED_AGAIN:
+                    m_runGas = sstoreUnchangedGas;
+                    break;
+                }
             }
 
             updateIOGas();

libaleth-interpreter/VM.h

@@ -30,8 +30,11 @@ struct VMSchedule
     static constexpr int64_t stepGas6 = 20;
     static constexpr int64_t sha3Gas = 30;
     static constexpr int64_t sha3WordGas = 6;
+    static constexpr int64_t sloadGas = 50;
     static constexpr int64_t sstoreSetGas = 20000;
     static constexpr int64_t sstoreResetGas = 5000;
+    static constexpr int64_t sstoreUnchangedGas = 200;
+    static constexpr int64_t sstoreRefundGas = 15000;
     static constexpr int64_t jumpdestGas = 1;
     static constexpr int64_t logGas = 375;
     static constexpr int64_t logDataGas = 8;
@@ -57,6 +60,7 @@ class VM
         evmc_revision _rev, const evmc_message* _msg, uint8_t const* _code, size_t _codeSize);
 
     uint64_t m_io_gas = 0;
+    int64_t m_gas_refunded = 0;
 private:
     const evmc_host_interface* m_host = nullptr;
     evmc_host_context* m_context = nullptr;

libaleth-interpreter/VMCalls.cpp

@@ -179,6 +179,9 @@ void VM::caseCall()
         m_SPP[0] = result.status_code == EVMC_SUCCESS ? 1 : 0;
         m_io_gas += result.gas_left;
 
+        // Aggregate refunds. This should be 0 in case of status code other than EVMC_SUCCESS.
+        m_gas_refunded += result.gas_refunded;
+
         if (result.release)
             result.release(&result);
     }

libethereum/ExtVM.cpp

@@ -154,15 +154,15 @@ CreateResult ExtVM::create(u256 _endowment, u256& io_gas, bytesConstRef _code, I
     return {transactionExceptionToEvmcStatusCode(e.getException()), e.takeOutput(), e.newAddress()};
 }
 
-void ExtVM::selfdestruct(Address _a)
+bool ExtVM::selfdestruct(Address _a)
 {
     // Why transfer is not used here? That caused a consensus issue before (see Quirk #2 in
     // http://martin.swende.se/blog/Ethereum_quirks_and_vulns.html). There is one test case
     // witnessing the current consensus
     // 'GeneralStateTests/stSystemOperationsTest/suicideSendEtherPostDeath.json'.
     m_s.addBalance(_a, m_s.balance(myAddress));
     m_s.setBalance(myAddress, 0);
-    ExtVMFace::selfdestruct(_a);
+    return ExtVMFace::selfdestruct(_a);
 }
 
 h256 ExtVM::blockHash(u256 _number)

libethereum/ExtVM.h

@@ -82,7 +82,7 @@ class ExtVM : public ExtVMFace
     }
 
     /// Selfdestruct the associated contract to the given address.
-    void selfdestruct(Address _a) final;
+    bool selfdestruct(Address _a) final;
 
     /// Return the EVM gas-price schedule for this execution context.
     EVMSchedule const& evmSchedule() const final { return m_evmSchedule; }

libevm/EVMC.cpp

@@ -84,7 +84,16 @@ owning_bytes_ref EVMC::exec(u256& io_gas, ExtVMFace& _ext, const OnOpFunc& _onOp
     EvmCHost host{_ext};
     auto r = execute(host, mode, msg, _ext.code.data(), _ext.code.size());
     // FIXME: Copy the output for now, but copyless version possible.
-    auto output = owning_bytes_ref{{&r.output_data[0], &r.output_data[r.output_size]}, 0, r.output_size};
+    auto output =
+        owning_bytes_ref{{&r.output_data[0], &r.output_data[r.output_size]}, 0, r.output_size};
+
+    if (_ext.sub.refunds != r.gas_refunded)
+    {
+        auto err_msg = "Incorrect gas refund. Expected: " + std::to_string(_ext.sub.refunds) +
+                       ". Got: " + std::to_string(r.gas_refunded);
+        cwarn << err_msg;
+        BOOST_THROW_EXCEPTION(InternalVMError{} << errinfo_comment(err_msg));
+    }
 
     switch (r.status_code)
     {

libevm/ExtVMFace.cpp

@@ -41,37 +41,100 @@ evmc_storage_status EvmCHost::set_storage(
         return EVMC_STORAGE_UNCHANGED;
 
     EVMSchedule const& schedule = m_extVM.evmSchedule();
-    auto status = EVMC_STORAGE_MODIFIED;
+
+    evmc_storage_status status;
+    int refunds = 0;
+    int expected = 0;
+
     u256 const originalValue = m_extVM.originalStorageValue(index);
-    if (originalValue == currentValue || !schedule.sstoreNetGasMetering())
+
+
+    if (originalValue == currentValue)
     {
         if (currentValue == 0)
             status = EVMC_STORAGE_ADDED;
         else if (newValue == 0)
         {
             status = EVMC_STORAGE_DELETED;
-            m_extVM.sub.refunds += schedule.sstoreRefundGas;
+            refunds = schedule.sstoreRefundGas;
+            expected += schedule.sstoreRefundGas;
         }
+        else
+            status = EVMC_STORAGE_MODIFIED;
     }
     else
     {
-        status = EVMC_STORAGE_MODIFIED_AGAIN;
+        if (originalValue == 0)
+        {
+            if (newValue == 0)
+            {
+                status = EVMC_DIRTY_ADDED_TO_DELETED;
+                refunds = schedule.sstoreSetGas - schedule.sstoreUnchangedGas;
+            }
+            else
+            {
+                status = EVMC_DIRTY_MODIFIED_AGAIN;
+            }
+        }
+        else
+        {
+            if (currentValue == 0)
+            {
+                if (newValue == originalValue)
+                {
+                    status = EVMC_DIRTY_DELETED_REVERTED;
+                    refunds = schedule.sstoreResetGas - schedule.sstoreUnchangedGas -
+                              schedule.sstoreRefundGas;
+                }
+                else
+                {
+                    status = EVMC_DIRTY_DELETED_TO_ADDED;
+                    refunds = -schedule.sstoreRefundGas;  // Can go negative.
+                }
+            }
+            else if (newValue == 0)
+            {
+                status = EVMC_DIRTY_MODIFIED_TO_DELETED;
+                refunds = schedule.sstoreRefundGas;
+            }
+            else if (newValue == originalValue)
+            {
+                status = EVMC_DIRTY_MODIFIED_REVERTED;
+                refunds = schedule.sstoreResetGas - schedule.sstoreUnchangedGas;
+            }
+            else
+            {
+                status = EVMC_DIRTY_MODIFIED_AGAIN;
+            }
+        }
+
+
         if (originalValue != 0)
         {
             if (currentValue == 0)
-                m_extVM.sub.refunds -= schedule.sstoreRefundGas;  // Can go negative.
+                expected += -(int)schedule.sstoreRefundGas;
             if (newValue == 0)
-                m_extVM.sub.refunds += schedule.sstoreRefundGas;
+                expected += (int)schedule.sstoreRefundGas;
         }
         if (originalValue == newValue)
         {
             if (originalValue == 0)
-                m_extVM.sub.refunds += schedule.sstoreSetGas - schedule.sstoreUnchangedGas;
+                expected += (int)(schedule.sstoreSetGas - schedule.sstoreUnchangedGas);
             else
-                m_extVM.sub.refunds += schedule.sstoreResetGas - schedule.sstoreUnchangedGas;
+            {
+                expected += (int)(schedule.sstoreResetGas - schedule.sstoreUnchangedGas);
+            }
         }
     }
 
+    if (refunds != expected)
+    {
+        std::cerr << refunds << " " << expected << "\n";
+        std::cerr << originalValue << " " << currentValue << " " << newValue << "\n";
+    }
+    assert(refunds == expected);
+
+    m_extVM.sub.refunds += refunds;
     m_extVM.setStore(index, newValue);  // Interface uses native endianness
 
     return status;
@@ -108,11 +171,11 @@ size_t EvmCHost::copy_code(evmc::address const& _addr, size_t _codeOffset, byte*
     return numToCopy;
 }
 
-void EvmCHost::selfdestruct(evmc::address const& _addr, evmc::address const& _beneficiary) noexcept
+bool EvmCHost::selfdestruct(evmc::address const& _addr, evmc::address const& _beneficiary) noexcept
 {
     (void)_addr;
     assert(fromEvmC(_addr) == m_extVM.myAddress);
-    m_extVM.selfdestruct(fromEvmC(_beneficiary));
+    return m_extVM.selfdestruct(fromEvmC(_beneficiary));
 }
 
 
@@ -214,6 +277,7 @@ evmc::result EvmCHost::call(evmc_message const& _msg) noexcept
     evmc_result evmcResult = {};
     evmcResult.status_code = result.status;
     evmcResult.gas_left = static_cast<int64_t>(params.gas);
+    evmcResult.gas_refunded = m_extVM.sub.refunds;
 
     // Pass the output to the EVM without a copy. The EVM will delete it
     // when finished with it.

libevm/ExtVMFace.h

@@ -230,10 +230,10 @@ class ExtVMFace
     ///
     /// @param beneficiary  The address of the account which will receive ETH
     ///                     from the selfdestructed account.
-    virtual void selfdestruct(Address beneficiary)
+    virtual bool selfdestruct(Address beneficiary)
     {
         (void)beneficiary;
-        sub.selfdestructs.insert(myAddress);
+        return sub.selfdestructs.insert(myAddress).second;
     }
 
     /// Create a new (contract) account.
@@ -300,7 +300,7 @@ class EvmCHost : public evmc::Host
     size_t copy_code(const evmc::address& _addr, size_t _codeOffset, uint8_t* _bufferData,
         size_t _bufferSize) const noexcept override;
 
-    void selfdestruct(
+    bool selfdestruct(
         const evmc::address& _addr, const evmc::address& _beneficiary) noexcept override;
 
     evmc::result call(const evmc_message& _msg) noexcept override;

test/tools/jsontests/vm.h

@@ -39,10 +39,10 @@ class FakeExtVM: public eth::ExtVMFace
     virtual void setStore(u256 _n, u256 _v) override { std::get<2>(addresses[myAddress])[_n] = _v; }
     virtual bool exists(Address _a) override { return !!addresses.count(_a); }
     virtual u256 balance(Address _a) override { return std::get<0>(addresses[_a]); }
-    virtual void selfdestruct(Address _a) override
+    virtual bool selfdestruct(Address _a) override
     {
         std::get<0>(addresses[_a]) += std::get<0>(addresses[myAddress]);
-        addresses.erase(myAddress);
+        return addresses.erase(myAddress) != 0;
     }
     virtual bytes const& codeAt(Address _a) override { return std::get<3>(addresses[_a]); }
     virtual size_t codeSizeAt(Address _a) override { return std::get<3>(addresses[_a]).size(); }