From a5a475ba92aff5da58d038a672261648ca3ffd04 Mon Sep 17 00:00:00 2001 From: tmohay <37158202+rain-on@users.noreply.github.com> Date: Thu, 11 Oct 2018 17:10:47 +1100 Subject: [PATCH] Fix COINBASE operation to return mining beneficiary (which may not match coinbase header in Clique) (#33) --- .../mainnet/MainnetTransactionProcessor.java | 2 ++ .../ethereum/vm/AbstractCallOperation.java | 1 + .../pantheon/ethereum/vm/MessageFrame.java | 24 +++++++++++++++++-- .../operations/AbstractCreateOperation.java | 1 + .../vm/operations/CoinbaseOperation.java | 2 +- .../ethereum/core/TestCodeExecutor.java | 1 + .../ethereum/vm/DebugOperationTracerTest.java | 4 +++- .../pantheon/ethereum/vm/VMReferenceTest.java | 1 + .../operations/ExtCodeHashOperationTest.java | 1 + 9 files changed, 33 insertions(+), 4 deletions(-) diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/mainnet/MainnetTransactionProcessor.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/mainnet/MainnetTransactionProcessor.java index 7d8a2ab4f2..36e3fba79d 100644 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/mainnet/MainnetTransactionProcessor.java +++ b/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(); } diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/AbstractCallOperation.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/AbstractCallOperation.java index 6c199d2447..b81b6ca912 100644 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/AbstractCallOperation.java +++ b/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); diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/MessageFrame.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/MessageFrame.java index 64081cfdfd..39939f62a5 100644 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/MessageFrame.java +++ b/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 messageFrameStack; + private final Address miningBeneficiary; // Miscellaneous fields. private final EnumSet exceptionalHaltReasons = @@ -229,7 +230,8 @@ public class MessageFrame { final ProcessableBlockHeader blockHeader, final int depth, final boolean isStatic, - final Consumer completer) { + final Consumer 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 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); } } } diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/operations/AbstractCreateOperation.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/operations/AbstractCreateOperation.java index 4c488dba57..ce266a4fb7 100644 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/operations/AbstractCreateOperation.java +++ b/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); diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/operations/CoinbaseOperation.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/operations/CoinbaseOperation.java index a79ff51ba2..ecdee3c09e 100644 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/operations/CoinbaseOperation.java +++ b/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)); } } diff --git a/ethereum/core/src/test-support/java/net/consensys/pantheon/ethereum/core/TestCodeExecutor.java b/ethereum/core/src/test-support/java/net/consensys/pantheon/ethereum/core/TestCodeExecutor.java index 955d4b5bd3..f8eccf6640 100644 --- a/ethereum/core/src/test-support/java/net/consensys/pantheon/ethereum/core/TestCodeExecutor.java +++ b/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); diff --git a/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/DebugOperationTracerTest.java b/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/DebugOperationTracerTest.java index 3d88954b3f..91992145c1 100644 --- a/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/DebugOperationTracerTest.java +++ b/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 setupStorageForCapture(final MessageFrame frame) { diff --git a/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/VMReferenceTest.java b/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/VMReferenceTest.java index c41aa91365..8dc2263691 100644 --- a/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/VMReferenceTest.java +++ b/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. diff --git a/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/operations/ExtCodeHashOperationTest.java b/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/operations/ExtCodeHashOperationTest.java index b86a7011b5..995a298bf1 100644 --- a/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/operations/ExtCodeHashOperationTest.java +++ b/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);