Fix COINBASE operation to return mining beneficiary (which may not match coinbase header in Clique) (#33)

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>

ethereum/core/src/main/java/net/consensys/pantheon/ethereum/mainnet/MainnetTransactionProcessor.java

@@ -198,6 +198,7 @@ public class MainnetTransactionProcessor implements TransactionProcessor {
               .blockHeader(blockHeader)
               .depth(0)
               .completer(c -> {})
+              .miningBeneficiary(miningBenficiary)
               .build();
 
     } else {
@@ -223,6 +224,7 @@ public class MainnetTransactionProcessor implements TransactionProcessor {
               .blockHeader(blockHeader)
               .depth(0)
               .completer(c -> {})
+              .miningBeneficiary(miningBenficiary)
               .build();
     }
 

ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/AbstractCallOperation.java

@@ -170,6 +170,7 @@ public abstract class AbstractCallOperation extends AbstractOperation {
             .depth(frame.getMessageStackDepth() + 1)
             .isStatic(isStatic(frame))
             .completer(child -> complete(frame, child))
+            .miningBeneficiary(frame.getMiningBeneficiary())
             .build();
 
     frame.getMessageFrameStack().addFirst(childFrame);

ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/MessageFrame.java

@@ -200,6 +200,7 @@ public class MessageFrame {
   private final ProcessableBlockHeader blockHeader;
   private final int depth;
   private final Deque<MessageFrame> messageFrameStack;
+  private final Address miningBeneficiary;
 
   // Miscellaneous fields.
   private final EnumSet<ExceptionalHaltReason> exceptionalHaltReasons =
@@ -229,7 +230,8 @@ public class MessageFrame {
       final ProcessableBlockHeader blockHeader,
       final int depth,
       final boolean isStatic,
-      final Consumer<MessageFrame> completer) {
+      final Consumer<MessageFrame> completer,
+      final Address miningBeneficiary) {
     this.type = type;
     this.blockchain = blockchain;
     this.messageFrameStack = messageFrameStack;
@@ -257,6 +259,7 @@ public class MessageFrame {
     this.state = State.NOT_STARTED;
     this.isStatic = isStatic;
     this.completer = completer;
+    this.miningBeneficiary = miningBeneficiary;
   }
 
   /**
@@ -771,6 +774,15 @@ public class MessageFrame {
     return exceptionalHaltReasons;
   }
 
+  /**
+   * Returns the current miningBeneficiary (aka coinbase)
+   *
+   * @return the current mining beneficiary
+   */
+  public Address getMiningBeneficiary() {
+    return miningBeneficiary;
+  }
+
   public Operation getCurrentOperation() {
     return currentOperation;
   }
@@ -799,6 +811,7 @@ public class MessageFrame {
     private int depth = -1;
     private boolean isStatic = false;
     private Consumer<MessageFrame> completer;
+    private Address miningBeneficiary;
 
     public Builder type(final Type type) {
       this.type = type;
@@ -890,6 +903,11 @@ public class MessageFrame {
       return this;
     }
 
+    public Builder miningBeneficiary(final Address miningBeneficiary) {
+      this.miningBeneficiary = miningBeneficiary;
+      return this;
+    }
+
     private void validate() {
       checkState(type != null, "Missing message frame type");
       checkState(blockchain != null, "Missing message frame blockchain");
@@ -908,6 +926,7 @@ public class MessageFrame {
       checkState(blockHeader != null, "Missing message frame block header");
       checkState(depth > -1, "Missing message frame depth");
       checkState(completer != null, "Missing message frame completer");
+      checkState(miningBeneficiary != null, "Missing mining beneficiary");
     }
 
     public MessageFrame build() {
@@ -931,7 +950,8 @@ public class MessageFrame {
           blockHeader,
           depth,
           isStatic,
-          completer);
+          completer,
+          miningBeneficiary);
     }
   }
 }

ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/operations/AbstractCreateOperation.java

@@ -108,6 +108,7 @@ public abstract class AbstractCreateOperation extends AbstractOperation {
             .blockHeader(frame.getBlockHeader())
             .depth(frame.getMessageStackDepth() + 1)
             .completer(child -> complete(frame, child))
+            .miningBeneficiary(frame.getMiningBeneficiary())
             .build();
 
     frame.getMessageFrameStack().addFirst(childFrame);

ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/operations/CoinbaseOperation.java

@@ -20,7 +20,7 @@ public class CoinbaseOperation extends AbstractOperation {
 
   @Override
   public void execute(final MessageFrame frame) {
-    final Address coinbase = frame.getBlockHeader().getCoinbase();
+    final Address coinbase = frame.getMiningBeneficiary();
     frame.pushStackItem(Words.fromAddress(coinbase));
   }
 }

ethereum/core/src/test-support/java/net/consensys/pantheon/ethereum/core/TestCodeExecutor.java

@@ -67,6 +67,7 @@ public class TestCodeExecutor {
             .blockHeader(blockHeader)
             .depth(0)
             .completer(c -> {})
+            .miningBeneficiary(blockHeader.coinbase)
             .build();
     messageFrameStack.addFirst(initialFrame);
 

ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/DebugOperationTracerTest.java

@@ -9,6 +9,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import net.consensys.pantheon.ethereum.core.Address;
+import net.consensys.pantheon.ethereum.core.AddressHelpers;
 import net.consensys.pantheon.ethereum.core.BlockHeaderTestFixture;
 import net.consensys.pantheon.ethereum.core.Gas;
 import net.consensys.pantheon.ethereum.core.MutableAccount;
@@ -205,7 +206,8 @@ public class DebugOperationTracerTest {
         .code(new Code())
         .blockHeader(new BlockHeaderTestFixture().buildHeader())
         .depth(DEPTH)
-        .completer(c -> {});
+        .completer(c -> {})
+        .miningBeneficiary(AddressHelpers.ofValue(0));
   }
 
   private Map<UInt256, UInt256> setupStorageForCapture(final MessageFrame frame) {

ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/VMReferenceTest.java

@@ -120,6 +120,7 @@ public class VMReferenceTest extends AbstractRetryingTest {
             .blockHeader(execEnv.getBlockHeader())
             .depth(execEnv.getDepth())
             .completer(c -> {})
+            .miningBeneficiary(execEnv.getBlockHeader().getCoinbase())
             .build();
 
     // This is normally set inside the containing message executing the code.

ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/operations/ExtCodeHashOperationTest.java

@@ -123,6 +123,7 @@ public class ExtCodeHashOperationTest {
             .code(new Code(BytesValue.EMPTY))
             .blockchain(blockchain)
             .completer(messageFrame -> {})
+            .miningBeneficiary(AddressHelpers.ofValue(0))
             .build();
 
     frame.pushStackItem(stackItem);