Better checking that correct opcode used with corresponding function code.

Added tests to cover above.

Java/src/main/java/org/ciyam/at/FunctionCode.java

@@ -1003,12 +1003,15 @@ public enum FunctionCode {
 	 */
 	API_PASSTHROUGH(0x0500, 0, false) {
 		@Override
-		public void preExecuteCheck(int paramCount, boolean returnValueExpected, MachineState state, short rawFunctionCode) throws ExecutionException {
+		public void preExecuteCheck(int paramCount, boolean returnValueExpected) throws ExecutionException {
-			state.getAPI().platformSpecificPreExecuteCheck(paramCount, returnValueExpected, state, rawFunctionCode);
+			// We don't know so skip checks at this point
 		}
 
 		@Override
 		protected void postCheckExecute(FunctionData functionData, MachineState state, short rawFunctionCode) throws ExecutionException {
+			// XXX somehow we need to call something like this:
+			// state.getAPI().platformSpecificPreExecuteCheck(functionData.paramCount, functionData.returnValueExpected, rawFunctionCode);
+
 			state.getAPI().platformSpecificPostCheckExecute(functionData, state, rawFunctionCode);
 		}
 	};
@@ -1034,7 +1037,7 @@ public enum FunctionCode {
 		return map.get((short) value);
 	}
 
-	public void preExecuteCheck(int paramCount, boolean returnValueExpected, MachineState state, short rawFunctionCode) throws ExecutionException {
+	public void preExecuteCheck(int paramCount, boolean returnValueExpected) throws ExecutionException {
 		if (paramCount != this.paramCount)
 			throw new IllegalFunctionCodeException(
 					"Passed paramCount (" + paramCount + ") does not match function's required paramCount (" + this.paramCount + ")");
@@ -1057,7 +1060,7 @@ public enum FunctionCode {
 	 */
 	public void execute(FunctionData functionData, MachineState state, short rawFunctionCode) throws ExecutionException {
 		// Check passed functionData against requirements of this function
-		preExecuteCheck(functionData.paramCount, functionData.returnValueExpected, state, rawFunctionCode);
+		preExecuteCheck(functionData.paramCount, functionData.returnValueExpected);
 
 		if (functionData.paramCount >= 1 && functionData.value1 == null)
 			throw new IllegalFunctionCodeException("Passed value1 is null but function has paramCount of (" + this.paramCount + ")");

Java/src/main/java/org/ciyam/at/OpCode.java

@@ -674,15 +674,23 @@ public enum OpCode {
 	 */
 	EXT_FUN(0x32, OpCodeParam.FUNC) {
 		@Override
-		protected void executeWithParams(MachineState state, Object... args) throws ExecutionException {
+		protected void preExecuteCheck(Object... args) throws ExecutionException {
 			short rawFunctionCode = (short) args[0];
 
 			FunctionCode functionCode = FunctionCode.valueOf(rawFunctionCode);
 
 			if (functionCode == null)
-				throw new IllegalFunctionCodeException("Unknown function code 0x" + String.format("%04x", rawFunctionCode) + " encountered at EXT_FUN");
+				throw new IllegalFunctionCodeException(String.format("Unknown function code 0x%04x encountered at %s",
+						rawFunctionCode, this.name()));
 
-			functionCode.preExecuteCheck(0, false, state, rawFunctionCode);
+			functionCode.preExecuteCheck(0, false);
+		}
+
+		@Override
+		protected void executeWithParams(MachineState state, Object... args) throws ExecutionException {
+			short rawFunctionCode = (short) args[0];
+
+			FunctionCode functionCode = FunctionCode.valueOf(rawFunctionCode);
 
 			FunctionData functionData = new FunctionData(false);
 
@@ -695,18 +703,25 @@ public enum OpCode {
 	 * <tt>func($addr)</tt>
 	 */
 	EXT_FUN_DAT(0x33, OpCodeParam.FUNC, OpCodeParam.SRC_ADDR) {
+		@Override
+		protected void preExecuteCheck(Object... args) throws ExecutionException {
+			short rawFunctionCode = (short) args[0];
+
+			FunctionCode functionCode = FunctionCode.valueOf(rawFunctionCode);
+
+			if (functionCode == null)
+				throw new IllegalFunctionCodeException(String.format("Unknown function code 0x%04x encountered at %s",
+						rawFunctionCode, this.name()));
+
+			functionCode.preExecuteCheck(1, false);
+		}
+
 		@Override
 		protected void executeWithParams(MachineState state, Object... args) throws ExecutionException {
 			short rawFunctionCode = (short) args[0];
 			int address = (int) args[1];
 
 			FunctionCode functionCode = FunctionCode.valueOf(rawFunctionCode);
-
-			if (functionCode == null)
-				throw new IllegalFunctionCodeException("Unknown function code 0x" + String.format("%04x", rawFunctionCode) + " encountered at EXT_FUN_DAT");
-
-			functionCode.preExecuteCheck(1, false, state, rawFunctionCode);
-
 			long value = state.dataByteBuffer.getLong(address);
 
 			FunctionData functionData = new FunctionData(value, false);
@@ -720,6 +735,19 @@ public enum OpCode {
 	 * <tt>func($addr1, $addr2)</tt>
 	 */
 	EXT_FUN_DAT_2(0x34, OpCodeParam.FUNC, OpCodeParam.SRC_ADDR, OpCodeParam.SRC_ADDR) {
+		@Override
+		protected void preExecuteCheck(Object... args) throws ExecutionException {
+			short rawFunctionCode = (short) args[0];
+
+			FunctionCode functionCode = FunctionCode.valueOf(rawFunctionCode);
+
+			if (functionCode == null)
+				throw new IllegalFunctionCodeException(String.format("Unknown function code 0x%04x encountered at %s",
+						rawFunctionCode, this.name()));
+
+			functionCode.preExecuteCheck(2, false);
+		}
+
 		@Override
 		protected void executeWithParams(MachineState state, Object... args) throws ExecutionException {
 			short rawFunctionCode = (short) args[0];
@@ -727,12 +755,6 @@ public enum OpCode {
 			int address2 = (int) args[2];
 
 			FunctionCode functionCode = FunctionCode.valueOf(rawFunctionCode);
-
-			if (functionCode == null)
-				throw new IllegalFunctionCodeException("Unknown function code 0x" + String.format("%04x", rawFunctionCode) + " encountered at EXT_FUN_DAT_2");
-
-			functionCode.preExecuteCheck(2, false, state, rawFunctionCode);
-
 			long value1 = state.dataByteBuffer.getLong(address1);
 			long value2 = state.dataByteBuffer.getLong(address2);
 
@@ -747,6 +769,19 @@ public enum OpCode {
 	 * <tt>@addr = func()</tt>
 	 */
 	EXT_FUN_RET(0x35, OpCodeParam.FUNC, OpCodeParam.DEST_ADDR) {
+		@Override
+		protected void preExecuteCheck(Object... args) throws ExecutionException {
+			short rawFunctionCode = (short) args[0];
+
+			FunctionCode functionCode = FunctionCode.valueOf(rawFunctionCode);
+
+			if (functionCode == null)
+				throw new IllegalFunctionCodeException(String.format("Unknown function code 0x%04x encountered at %s",
+						rawFunctionCode, this.name()));
+
+			functionCode.preExecuteCheck(0, true);
+		}
+
 		@Override
 		protected void executeWithParams(MachineState state, Object... args) throws ExecutionException {
 			short rawFunctionCode = (short) args[0];
@@ -754,11 +789,6 @@ public enum OpCode {
 
 			FunctionCode functionCode = FunctionCode.valueOf(rawFunctionCode);
 
-			if (functionCode == null)
-				throw new IllegalFunctionCodeException("Unknown function code 0x" + String.format("%04x", rawFunctionCode) + " encountered at EXT_FUN_RET");
-
-			functionCode.preExecuteCheck(0, true, state, rawFunctionCode);
-
 			FunctionData functionData = new FunctionData(true);
 
 			functionCode.execute(functionData, state, rawFunctionCode);
@@ -775,6 +805,19 @@ public enum OpCode {
 	 * <tt>@addr1 = func($addr2)</tt>
 	 */
 	EXT_FUN_RET_DAT(0x36, OpCodeParam.FUNC, OpCodeParam.DEST_ADDR, OpCodeParam.SRC_ADDR) {
+		@Override
+		protected void preExecuteCheck(Object... args) throws ExecutionException {
+			short rawFunctionCode = (short) args[0];
+
+			FunctionCode functionCode = FunctionCode.valueOf(rawFunctionCode);
+
+			if (functionCode == null)
+				throw new IllegalFunctionCodeException(String.format("Unknown function code 0x%04x encountered at %s",
+						rawFunctionCode, this.name()));
+
+			functionCode.preExecuteCheck(1, true);
+		}
+
 		@Override
 		protected void executeWithParams(MachineState state, Object... args) throws ExecutionException {
 			short rawFunctionCode = (short) args[0];
@@ -782,12 +825,6 @@ public enum OpCode {
 			int address2 = (int) args[2];
 
 			FunctionCode functionCode = FunctionCode.valueOf(rawFunctionCode);
-
-			if (functionCode == null)
-				throw new IllegalFunctionCodeException("Unknown function code 0x" + String.format("%04x", rawFunctionCode) + " encountered at EXT_FUN_RET_DAT");
-
-			functionCode.preExecuteCheck(1, true, state, rawFunctionCode);
-
 			long value = state.dataByteBuffer.getLong(address2);
 
 			FunctionData functionData = new FunctionData(value, true);
@@ -806,6 +843,19 @@ public enum OpCode {
 	 * <tt>@addr1 = func($addr2, $addr3)</tt>
 	 */
 	EXT_FUN_RET_DAT_2(0x37, OpCodeParam.FUNC, OpCodeParam.DEST_ADDR, OpCodeParam.SRC_ADDR, OpCodeParam.SRC_ADDR) {
+		@Override
+		protected void preExecuteCheck(Object... args) throws ExecutionException {
+			short rawFunctionCode = (short) args[0];
+
+			FunctionCode functionCode = FunctionCode.valueOf(rawFunctionCode);
+
+			if (functionCode == null)
+				throw new IllegalFunctionCodeException(String.format("Unknown function code 0x%04x encountered at %s",
+						rawFunctionCode, this.name()));
+
+			functionCode.preExecuteCheck(2, true);
+		}
+
 		@Override
 		protected void executeWithParams(MachineState state, Object... args) throws ExecutionException {
 			short rawFunctionCode = (short) args[0];
@@ -814,13 +864,6 @@ public enum OpCode {
 			int address3 = (int) args[3];
 
 			FunctionCode functionCode = FunctionCode.valueOf(rawFunctionCode);
-
-			if (functionCode == null)
-				throw new IllegalFunctionCodeException(
-						"Unknown function code 0x" + String.format("%04x", rawFunctionCode) + " encountered at EXT_FUN_RET_DAT_2");
-
-			functionCode.preExecuteCheck(2, true, state, rawFunctionCode);
-
 			long value1 = state.dataByteBuffer.getLong(address2);
 			long value2 = state.dataByteBuffer.getLong(address3);
 
@@ -869,13 +912,21 @@ public enum OpCode {
 	 */
 	protected abstract void executeWithParams(MachineState state, Object... args) throws ExecutionException;
 
+	protected void preExecuteCheck(Object... args) throws ExecutionException {
+		/* Can be overridden on a per-opcode basis */
+	}
+
 	/* package */ void execute(MachineState state) throws ExecutionException {
 		List<Object> args = new ArrayList<>();
 
 		for (OpCodeParam param : this.params)
 			args.add(param.fetch(state.codeByteBuffer, state.dataByteBuffer));
 
-		this.executeWithParams(state, args.toArray());
+		Object[] argsArray = args.toArray();
+
+		preExecuteCheck(argsArray);
+
+		this.executeWithParams(state, argsArray);
 	}
 
 	public static int calcOffset(ByteBuffer byteBuffer, Integer branchTarget) {
@@ -895,9 +946,9 @@ public enum OpCode {
 
 		for (int i = 0; i < this.params.length; ++i)
 			try {
-				byteBuffer.put(this.params[i].compile(args[i]));
+				byteBuffer.put(this.params[i].compile(this, args[i]));
 			} catch (ClassCastException e) {
-				throw new CompilationException(String.format("%s arg[%d] could not coerced to required type", this.name(), i));
+				throw new CompilationException(String.format("%s arg[%d] could not coerced to required type: %s", this.name(), i, e.getMessage()));
 			}
 
 		byteBuffer.flip();

Java/src/main/java/org/ciyam/at/OpCodeParam.java

@@ -1,7 +1,6 @@
 package org.ciyam.at;
 
 import java.nio.ByteBuffer;
-import java.util.function.Function;
 
 enum OpCodeParam {
 
@@ -148,60 +147,68 @@ enum OpCodeParam {
 		}
 	};
 
-	private final Function<? super Object, byte[]> compiler;
+	@FunctionalInterface
+	private interface Compiler {
+		byte[] compile(OpCode opCode, Object arg);
+	}
+	private final Compiler compiler;
 
-	private OpCodeParam(Function<? super Object, byte[]> compiler) {
+	private OpCodeParam(Compiler compiler) {
 		this.compiler = compiler;
 	}
 
 	public abstract Object fetch(ByteBuffer codeByteBuffer, ByteBuffer dataByteBuffer) throws ExecutionException;
 
-	private static byte[] compileByte(Object o) {
+	private static byte[] compileByte(OpCode opcode, Object arg) {
 		// Highly likely to be an Integer, so try that first
 		try {
-			int intValue = (int) o;
+			int intValue = (int) arg;
 			if (intValue < Byte.MIN_VALUE || intValue > Byte.MAX_VALUE)
 				throw new ClassCastException("Value too large to compile to byte");
 
 			return new byte[] { (byte) intValue };
 		} catch (ClassCastException e) {
 			// Try again using Byte
-			return new byte[] { (byte) o };
+			return new byte[] { (byte) arg };
 		}
 	}
 
-	private static byte[] compileShort(Object o) {
+	private static byte[] compileShort(OpCode opcode, Object arg) {
-		short s = (short) o;
+		short s = (short) arg;
 		return new byte[] { (byte) (s >>> 8), (byte) (s) };
 	}
 
-	private static byte[] compileInt(Object o) {
+	private static byte[] compileInt(OpCode opcode, Object arg) {
-		return MachineState.toByteArray((int) o);
+		return MachineState.toByteArray((int) arg);
 	}
 
-	private static byte[] compileLong(Object o) {
+	private static byte[] compileLong(OpCode opcode, Object arg) {
 		// Highly likely to be a Long, so try that first
 		try {
-			return MachineState.toByteArray((long) o);
+			return MachineState.toByteArray((long) arg);
 		} catch (ClassCastException e) {
 			// Try again using Integer
-			return MachineState.toByteArray((long)(int) o);
+			return MachineState.toByteArray((long)(int) arg);
 		}
 	}
 
-	private static byte[] compileFunc(Object o) {
+	private static byte[] compileFunc(OpCode opcode, Object arg) {
 		try {
-			FunctionCode func = (FunctionCode) o;
+			FunctionCode func = (FunctionCode) arg;
-			return compileShort(func.value);
+			opcode.preExecuteCheck(func.value);
+			return compileShort(opcode, func.value);
 		} catch (ClassCastException e) {
 			// Couldn't cast to FunctionCode,
 			// but try Short in case caller is using API-PASSTHROUGH range
-			return compileShort(o);
+			return compileShort(opcode, arg);
+		} catch (ExecutionException e) {
+			// Wrong opcode for this function
+			throw new ClassCastException("Wrong opcode for this function");
 		}
 	}
 
-	protected byte[] compile(Object arg) {
+	protected byte[] compile(OpCode opcode, Object arg) {
-		return this.compiler.apply(arg);
+		return this.compiler.compile(opcode, arg);
 	}
 
 	public String disassemble(ByteBuffer codeByteBuffer, ByteBuffer dataByteBuffer, int postOpcodeProgramCounter) throws ExecutionException {

Java/src/test/java/org/ciyam/at/CompileTests.java

@@ -155,15 +155,18 @@ public class CompileTests {
 		final int addrHashPart2 = addrCounter++;
 		final int addrHashPart3 = addrCounter++;
 		final int addrHashPart4 = addrCounter++;
+		final int addrHashIndex = addrCounter++;
 		final int addrAddressPart1 = addrCounter++;
 		final int addrAddressPart2 = addrCounter++;
 		final int addrAddressPart3 = addrCounter++;
 		final int addrAddressPart4 = addrCounter++;
+		final int addrAddressIndex = addrCounter++;
 		final int addrRefundMinutes = addrCounter++;
 		final int addrHashTempIndex = addrCounter++;
 		final int addrHashTempLength = addrCounter++;
 		final int addrInitialPayoutAmount = addrCounter++;
 		final int addrExpectedTxType = addrCounter++;
+		final int addrAddressTempIndex = addrCounter++;
 		// Variables
 		final int addrRefundTimestamp = addrCounter++;
 		final int addrLastTimestamp = addrCounter++;
@@ -195,7 +198,7 @@ public class CompileTests {
 			codeByteBuffer.put(OpCode.EXT_FUN_RET_DAT_2.compile(FunctionCode.ADD_MINUTES_TO_TIMESTAMP, addrRefundTimestamp, addrLastTimestamp, addrRefundMinutes));
 
 			// Load recipient's address into B register
-			codeByteBuffer.put(OpCode.EXT_FUN_DAT.compile(FunctionCode.SET_B_IND, addrAddressPart1));
+			codeByteBuffer.put(OpCode.EXT_FUN_DAT.compile(FunctionCode.SET_B_IND, addrAddressIndex));
 			// Send initial payment to recipient so they have enough funds to message AT if all goes well
 			codeByteBuffer.put(OpCode.EXT_FUN_DAT.compile(FunctionCode.PAY_TO_ADDRESS_IN_B, addrInitialPayoutAmount));
 
@@ -237,8 +240,8 @@ public class CompileTests {
 
 			// Extract sender address from transaction into B register
 			codeByteBuffer.put(OpCode.EXT_FUN.compile(FunctionCode.PUT_ADDRESS_FROM_TX_IN_A_INTO_B));
-			// Save B register into data segment starting at addrAddressTemp1
+			// Save B register into data segment starting at addrAddressTemp1 (as pointed to by addrAddressTempIndex)
-			codeByteBuffer.put(OpCode.EXT_FUN_RET.compile(FunctionCode.GET_B_IND, addrAddressTemp1));
+			codeByteBuffer.put(OpCode.EXT_FUN_DAT.compile(FunctionCode.GET_B_IND, addrAddressTempIndex));
 			// Compare each part of transaction's sender's address with expected address. If they don't match, look for another transaction.
 			codeByteBuffer.put(OpCode.BNE_DAT.compile(addrAddressTemp1, addrAddressPart1, calcOffset(codeByteBuffer, labelTxLoop)));
 			codeByteBuffer.put(OpCode.BNE_DAT.compile(addrAddressTemp2, addrAddressPart2, calcOffset(codeByteBuffer, labelTxLoop)));
@@ -249,10 +252,10 @@ public class CompileTests {
 
 			// Extract message from transaction into B register
 			codeByteBuffer.put(OpCode.EXT_FUN.compile(FunctionCode.PUT_MESSAGE_FROM_TX_IN_A_INTO_B));
-			// Save B register into data segment starting at addrHashTemp1
+			// Save B register into data segment starting at addrHashTemp1 (as pointed to by addrHashTempIndex)
-			codeByteBuffer.put(OpCode.EXT_FUN_RET.compile(FunctionCode.GET_B_IND, addrHashTemp1));
+			codeByteBuffer.put(OpCode.EXT_FUN_DAT.compile(FunctionCode.GET_B_IND, addrHashTempIndex));
-			// Load B register with expected hash result
+			// Load B register with expected hash result (as pointed to by addrHashIndex)
-			codeByteBuffer.put(OpCode.EXT_FUN_DAT.compile(FunctionCode.SET_B_IND, addrHashPart1));
+			codeByteBuffer.put(OpCode.EXT_FUN_DAT.compile(FunctionCode.SET_B_IND, addrHashIndex));
 			// Perform HASH160 using source data at addrHashTemp1 through addrHashTemp4. (Location and length specified via addrHashTempIndex and addrHashTemplength).
 			// Save the equality result (1 if they match, 0 otherwise) into addrComparator.
 			codeByteBuffer.put(OpCode.EXT_FUN_RET_DAT_2.compile(FunctionCode.CHECK_HASH160_WITH_B, addrComparator, addrHashTempIndex, addrHashTempLength));
@@ -261,8 +264,8 @@ public class CompileTests {
 
 			/* Success! Pay balance to intended recipient */
 
-			// Load B register with intended recipient address.
+			// Load B register with intended recipient address (as pointed to by addrAddressIndex)
-			codeByteBuffer.put(OpCode.EXT_FUN_DAT.compile(FunctionCode.SET_B_IND, addrAddressPart1));
+			codeByteBuffer.put(OpCode.EXT_FUN_DAT.compile(FunctionCode.SET_B_IND, addrAddressIndex));
 			// Pay AT's balance to recipient
 			codeByteBuffer.put(OpCode.EXT_FUN.compile(FunctionCode.PAY_ALL_TO_ADDRESS_IN_B));
 			// We're finished forever

Java/src/test/java/org/ciyam/at/FunctionCodeTests.java

@@ -50,6 +50,37 @@ public class FunctionCodeTests extends ExecutableTest {
 		assertFalse(state.hadFatalError());
 	}
 
+	@Test
+	public void testIncorrectFunctionCodeOldStyle() throws ExecutionException {
+		// SET_B_IND should be EXT_FUN_DAT not EXT_FUN_RET
+		codeByteBuffer.put(OpCode.EXT_FUN_RET.value).putShort(FunctionCode.SET_B_IND.value).putInt(0);
+		codeByteBuffer.put(OpCode.FIN_IMD.value);
+
+		execute(true);
+
+		assertTrue(state.isFinished());
+		assertTrue(state.hadFatalError());
+	}
+
+	@Test
+	public void testIncorrectFunctionCodeNewStyle() throws ExecutionException {
+		try {
+			// SET_B_IND should be EXT_FUN_DAT not EXT_FUN_RET
+			codeByteBuffer.put(OpCode.EXT_FUN_RET.compile(FunctionCode.SET_B_IND, 0));
+			codeByteBuffer.put(OpCode.FIN_IMD.compile());
+
+			execute(true);
+
+			assertTrue(state.isFinished());
+			assertTrue(state.hadFatalError());
+		} catch (CompilationException e) {
+			// Expected behaviour!
+			return;
+		}
+
+		fail("Compilation was expected to fail as EXT_FUN_RET is incorrect for SET_B_IND");
+	}
+
 	@Test
 	public void testInvalidFunctionCode() throws ExecutionException {
 		codeByteBuffer.put(OpCode.EXT_FUN.value).putShort((short) 0xaaaa);