From abcbe21f33155bb5e66b978403bc439548f9915b Mon Sep 17 00:00:00 2001 From: orogvany Date: Tue, 18 Dec 2018 09:30:29 -0800 Subject: [PATCH 1/5] VM: Remove fee for VM calls Update API to reflect --- .../semux/api/util/TransactionBuilder.java | 7 +++ .../java/org/semux/api/v2/SemuxApiImpl.java | 16 +++--- src/main/java/org/semux/core/Amount.java | 4 ++ .../org/semux/core/TransactionExecutor.java | 53 +++++++++++-------- .../org/semux/api/swagger/v2.2.0.json | 27 ---------- src/test/java/org/semux/core/VmTest.java | 7 ++- 6 files changed, 54 insertions(+), 60 deletions(-) diff --git a/src/main/java/org/semux/api/util/TransactionBuilder.java b/src/main/java/org/semux/api/util/TransactionBuilder.java index 8b2b92c4a..fd6f5cf80 100644 --- a/src/main/java/org/semux/api/util/TransactionBuilder.java +++ b/src/main/java/org/semux/api/util/TransactionBuilder.java @@ -164,6 +164,13 @@ public TransactionBuilder withValue(String value) { return this; // ignore the provided parameter } + if (type == TransactionType.CREATE) { + if (value != null && !value.isEmpty()) { + throw new IllegalArgumentException("Parameter `value` is not needed for CREATE transaction"); + } + return this; // ignore the provided parameter + } + if (value == null) { throw new IllegalArgumentException("Parameter `value` is required"); } diff --git a/src/main/java/org/semux/api/v2/SemuxApiImpl.java b/src/main/java/org/semux/api/v2/SemuxApiImpl.java index 997dfd93a..ea5b23e28 100644 --- a/src/main/java/org/semux/api/v2/SemuxApiImpl.java +++ b/src/main/java/org/semux/api/v2/SemuxApiImpl.java @@ -744,17 +744,16 @@ public Response transfer(String from, String to, String value, String fee, Strin } @Override - public Response create(String from, String value, String data, String gasPrice, String gas, String fee, - String nonce, Boolean validateNonce) { - return doTransaction(TransactionType.CREATE, from, null, value, fee, nonce, validateNonce, data, gasPrice, + public Response create(String from, String data, String gasPrice, String gas, String nonce, Boolean validateNonce) { + return doTransaction(TransactionType.CREATE, from, null, null, null, nonce, validateNonce, data, gasPrice, gas); } @Override - public Response call(String from, String to, String value, String gasPrice, String gas, String fee, + public Response call(String from, String to, String value, String gasPrice, String gas, String nonce, Boolean validateNonce, String data, Boolean local) { if (local) { - Transaction tx = getTransaction(TransactionType.CALL, from, to, value, fee, nonce, validateNonce, data, + Transaction tx = getTransaction(TransactionType.CALL, from, to, value, null, nonce, validateNonce, data, gasPrice, gas); SemuxTransaction transaction = new SemuxTransaction(tx); @@ -777,7 +776,7 @@ public Response call(String from, String to, String value, String gasPrice, Stri return success(resp); } } else { - return doTransaction(TransactionType.CALL, from, to, value, fee, nonce, validateNonce, data, gasPrice, + return doTransaction(TransactionType.CALL, from, to, value, null, nonce, validateNonce, data, gasPrice, gas); } } @@ -922,10 +921,12 @@ private Transaction getTransaction(TransactionType type, String from, String to, .withFrom(from) .withTo(to) .withValue(value) - .withFee(fee, true) .withData(data); + if (type == TransactionType.CREATE || type == TransactionType.CALL) { transactionBuilder.withGasPrice(gasPrice).withGas(gas); + } else { + transactionBuilder.withFee(fee, true); } if (nonce != null) { @@ -943,7 +944,6 @@ private Transaction getTransaction(TransactionType type, String from, String to, throw new IllegalArgumentException("Invalid transaction nonce."); } return tx; - } /** diff --git a/src/main/java/org/semux/core/Amount.java b/src/main/java/org/semux/core/Amount.java index 5e16779cd..0aa6275e5 100644 --- a/src/main/java/org/semux/core/Amount.java +++ b/src/main/java/org/semux/core/Amount.java @@ -45,6 +45,10 @@ public Amount of(long a) { return new Amount(Math.multiplyExact(a, factor)); } + public Amount ofGas(long gas, long gasPrice) { + return new Amount(Math.multiplyExact(gas, gasPrice)); + } + public BigDecimal toDecimal(Amount a, int scale) { BigDecimal $nano = BigDecimal.valueOf(a.nano); return $nano.movePointLeft(exp).setScale(scale, FLOOR); diff --git a/src/main/java/org/semux/core/TransactionExecutor.java b/src/main/java/org/semux/core/TransactionExecutor.java index aa51db634..12e8ad1c1 100644 --- a/src/main/java/org/semux/core/TransactionExecutor.java +++ b/src/main/java/org/semux/core/TransactionExecutor.java @@ -115,10 +115,19 @@ public List execute(List txs, AccountState as, D continue; } - // check fee - if (fee.lt(config.minTransactionFee())) { - result.setCode(Code.INVALID_FEE); - continue; + boolean isVmCall = type == TransactionType.CREATE || type == TransactionType.CALL; + + // check fee (call and create use gas instead) + if (isVmCall) { + if (fee.gt0()) { + result.setCode(Code.INVALID_FEE); + continue; + } + } else { + if (fee.lt(config.minTransactionFee())) { + result.setCode(Code.INVALID_FEE); + continue; + } } // check data length @@ -197,25 +206,27 @@ public List execute(List txs, AccountState as, D case CALL: case CREATE: long maxGasFee = tx.getGas() * tx.getGasPrice(); - Amount maxCost = sum(sum(value, fee), Unit.NANO_SEM.of(maxGasFee)); - if (maxCost.lte(available)) { - // VM calls still use fees - as.adjustAvailable(from, neg(sum(value, fee))); + Amount maxCost = sum(value, Unit.NANO_SEM.of(maxGasFee)); + if (available.equals(maxCost)) { + result.setCode(Code.INSUFFICIENT_AVAILABLE); + break; + } - if (tx.getGas() > config.vmMaxBlockGasLimit()) { - result.setCode(Code.INVALID_GAS); - } else if (block == null) { - // workaround for pending manager so it doesn't execute these - // we charge gas later - as.increaseNonce(from); - result.setCode(Code.SUCCESS); - } else { - executeVmTransaction(result, tx, as, block, gasUsedInBlock); - gasUsedInBlock += result.getGasUsed(); - } + // VM calls can still take values + as.adjustAvailable(from, neg(value)); + + if (tx.getGas() > config.vmMaxBlockGasLimit()) { + result.setCode(Code.INVALID_GAS); + } else if (block == null) { + // workaround for pending manager so it doesn't execute these + // we charge gas later + as.increaseNonce(from); + result.setCode(Code.SUCCESS); } else { - result.setCode(Code.INSUFFICIENT_AVAILABLE); + executeVmTransaction(result, tx, as, block, gasUsedInBlock); + gasUsedInBlock += result.getGasUsed(); } + break; default: @@ -226,7 +237,7 @@ public List execute(List txs, AccountState as, D // increase nonce if success // creates and calls increase their own nonces internal to VM - if (result.getCode().isAccepted() && type != TransactionType.CREATE && type != TransactionType.CALL) { + if (result.getCode().isAccepted() && !isVmCall) { as.increaseNonce(from); } } diff --git a/src/main/resources/org/semux/api/swagger/v2.2.0.json b/src/main/resources/org/semux/api/swagger/v2.2.0.json index 68332273b..dbf0572a0 100644 --- a/src/main/resources/org/semux/api/swagger/v2.2.0.json +++ b/src/main/resources/org/semux/api/swagger/v2.2.0.json @@ -1059,24 +1059,6 @@ "type" : "string", "pattern" : "^(0x)?[0-9a-fA-F]{40}$" }, - { - "name" : "value", - "in" : "query", - "description" : "Amount of SEM to transfer in nano SEM", - "required" : true, - "type" : "string", - "format" : "int64", - "pattern" : "^\\d+$" - }, - { - "name" : "fee", - "in" : "query", - "description" : "Transaction fee in nano SEM, default to minimum fee if omitted", - "required" : false, - "type" : "string", - "format" : "int64", - "pattern" : "^\\d+$" - }, { "name" : "nonce", "in" : "query", @@ -1173,15 +1155,6 @@ "format" : "int64", "pattern" : "^\\d+$" }, - { - "name" : "fee", - "in" : "query", - "description" : "Transaction fee in nano SEM, default to minimum fee if omitted", - "required" : false, - "type" : "string", - "format" : "int64", - "pattern" : "^\\d+$" - }, { "name" : "nonce", "in" : "query", diff --git a/src/test/java/org/semux/core/VmTest.java b/src/test/java/org/semux/core/VmTest.java index 7d937ba42..2f9bb982a 100644 --- a/src/test/java/org/semux/core/VmTest.java +++ b/src/test/java/org/semux/core/VmTest.java @@ -64,7 +64,6 @@ public void testCall() { byte[] from = key.toAddress(); byte[] to = Bytes.random(20); Amount value = NANO_SEM.of(5); - Amount fee = config.minTransactionFee(); long nonce = as.getAccount(from).getNonce(); long timestamp = TimeUtil.currentTimeMillis(); @@ -84,7 +83,7 @@ public void testCall() { long gas = 30000; long gasPrice = 1; - Transaction tx = new Transaction(network, type, to, value, fee, nonce, timestamp, data, gas, gasPrice); + Transaction tx = new Transaction(network, type, to, value, Amount.ZERO, nonce, timestamp, data, gas, gasPrice); tx.sign(key); assertTrue(tx.validate(network)); @@ -117,7 +116,6 @@ public void testCreate() { byte[] from = key.toAddress(); byte[] to = Bytes.EMPTY_ADDRESS; Amount value = NANO_SEM.of(0); - Amount fee = config.minTransactionFee(); long nonce = as.getAccount(from).getNonce(); long timestamp = TimeUtil.currentTimeMillis(); @@ -134,7 +132,8 @@ public void testCreate() { long gas = 1000000; long gasPrice = 1; - Transaction tx = new Transaction(network, type, to, value, fee, nonce, timestamp, create, gas, gasPrice); + Transaction tx = new Transaction(network, type, to, value, Amount.ZERO, nonce, timestamp, create, gas, + gasPrice); tx.sign(key); assertTrue(tx.validate(network)); From 0fea4ca4b60a284fe13b81c3053c67d26a188b56 Mon Sep 17 00:00:00 2001 From: orogvany Date: Tue, 18 Dec 2018 09:34:43 -0800 Subject: [PATCH 2/5] API: Zero for fee to ensure tx marshalls correctly, use 0 for fee. --- src/main/java/org/semux/api/v2/SemuxApiImpl.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/semux/api/v2/SemuxApiImpl.java b/src/main/java/org/semux/api/v2/SemuxApiImpl.java index ea5b23e28..1a702df04 100644 --- a/src/main/java/org/semux/api/v2/SemuxApiImpl.java +++ b/src/main/java/org/semux/api/v2/SemuxApiImpl.java @@ -745,7 +745,7 @@ public Response transfer(String from, String to, String value, String fee, Strin @Override public Response create(String from, String data, String gasPrice, String gas, String nonce, Boolean validateNonce) { - return doTransaction(TransactionType.CREATE, from, null, null, null, nonce, validateNonce, data, gasPrice, + return doTransaction(TransactionType.CREATE, from, null, null, "0", nonce, validateNonce, data, gasPrice, gas); } @@ -753,7 +753,7 @@ public Response create(String from, String data, String gasPrice, String gas, St public Response call(String from, String to, String value, String gasPrice, String gas, String nonce, Boolean validateNonce, String data, Boolean local) { if (local) { - Transaction tx = getTransaction(TransactionType.CALL, from, to, value, null, nonce, validateNonce, data, + Transaction tx = getTransaction(TransactionType.CALL, from, to, value, "0", nonce, validateNonce, data, gasPrice, gas); SemuxTransaction transaction = new SemuxTransaction(tx); @@ -776,7 +776,7 @@ public Response call(String from, String to, String value, String gasPrice, Stri return success(resp); } } else { - return doTransaction(TransactionType.CALL, from, to, value, null, nonce, validateNonce, data, gasPrice, + return doTransaction(TransactionType.CALL, from, to, value, "0", nonce, validateNonce, data, gasPrice, gas); } } @@ -920,13 +920,12 @@ private Transaction getTransaction(TransactionType type, String from, String to, .withType(type) .withFrom(from) .withTo(to) + .withFee(fee, true) .withValue(value) .withData(data); if (type == TransactionType.CREATE || type == TransactionType.CALL) { transactionBuilder.withGasPrice(gasPrice).withGas(gas); - } else { - transactionBuilder.withFee(fee, true); } if (nonce != null) { From e5de6fdd3cc3afe73c4786551479caeef94c8c33 Mon Sep 17 00:00:00 2001 From: orogvany Date: Sun, 13 Jan 2019 23:41:43 -0800 Subject: [PATCH 3/5] API: Fix amount default value Also bump min gas fee --- src/main/java/org/semux/api/util/TransactionBuilder.java | 1 + src/main/java/org/semux/config/AbstractConfig.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/semux/api/util/TransactionBuilder.java b/src/main/java/org/semux/api/util/TransactionBuilder.java index fd6f5cf80..70aba2c09 100644 --- a/src/main/java/org/semux/api/util/TransactionBuilder.java +++ b/src/main/java/org/semux/api/util/TransactionBuilder.java @@ -263,6 +263,7 @@ public Transaction buildUnsigned() { } if (type == TransactionType.CREATE) { to = Bytes.EMPTY_ADDRESS; + value = Amount.ZERO; } return new Transaction( diff --git a/src/main/java/org/semux/config/AbstractConfig.java b/src/main/java/org/semux/config/AbstractConfig.java index 968ae3743..1892089c2 100644 --- a/src/main/java/org/semux/config/AbstractConfig.java +++ b/src/main/java/org/semux/config/AbstractConfig.java @@ -121,7 +121,7 @@ public abstract class AbstractConfig implements Config { protected int vmInitHeapSize = 128; protected int vmBlockGasLimit = 999_999; protected int vmMaxBlockGasLimit = 9_999_999; - protected int vmMinGasPrice = 1; + protected int vmMinGasPrice = 10; // ========================= // UI From 14a97ec83fe50fdd88a7f0b1d8a84c960e823599 Mon Sep 17 00:00:00 2001 From: orogvany Date: Wed, 16 Jan 2019 19:50:15 -0800 Subject: [PATCH 4/5] Core: Update fee handling for VM --- src/main/java/org/semux/config/AbstractConfig.java | 2 +- src/main/java/org/semux/core/TransactionExecutor.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/semux/config/AbstractConfig.java b/src/main/java/org/semux/config/AbstractConfig.java index 1892089c2..968ae3743 100644 --- a/src/main/java/org/semux/config/AbstractConfig.java +++ b/src/main/java/org/semux/config/AbstractConfig.java @@ -121,7 +121,7 @@ public abstract class AbstractConfig implements Config { protected int vmInitHeapSize = 128; protected int vmBlockGasLimit = 999_999; protected int vmMaxBlockGasLimit = 9_999_999; - protected int vmMinGasPrice = 10; + protected int vmMinGasPrice = 1; // ========================= // UI diff --git a/src/main/java/org/semux/core/TransactionExecutor.java b/src/main/java/org/semux/core/TransactionExecutor.java index 12e8ad1c1..c241fc089 100644 --- a/src/main/java/org/semux/core/TransactionExecutor.java +++ b/src/main/java/org/semux/core/TransactionExecutor.java @@ -119,7 +119,7 @@ public List execute(List txs, AccountState as, D // check fee (call and create use gas instead) if (isVmCall) { - if (fee.gt0()) { + if (fee.lt(Amount.ZERO)) { result.setCode(Code.INVALID_FEE); continue; } From 454f0f98ac9c060a589041f1cd9d67249b2331df Mon Sep 17 00:00:00 2001 From: orogvany Date: Wed, 16 Jan 2019 19:55:03 -0800 Subject: [PATCH 5/5] Core: fix available balance check --- src/main/java/org/semux/core/TransactionExecutor.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/semux/core/TransactionExecutor.java b/src/main/java/org/semux/core/TransactionExecutor.java index c241fc089..b15afd33e 100644 --- a/src/main/java/org/semux/core/TransactionExecutor.java +++ b/src/main/java/org/semux/core/TransactionExecutor.java @@ -206,14 +206,16 @@ public List execute(List txs, AccountState as, D case CALL: case CREATE: long maxGasFee = tx.getGas() * tx.getGasPrice(); - Amount maxCost = sum(value, Unit.NANO_SEM.of(maxGasFee)); - if (available.equals(maxCost)) { + + Amount maxCost = sum(sum(value, fee), Unit.NANO_SEM.of(maxGasFee)); + + if (available.lt(maxCost)) { result.setCode(Code.INSUFFICIENT_AVAILABLE); break; } - // VM calls can still take values - as.adjustAvailable(from, neg(value)); + // VM calls can have fees/values set. + as.adjustAvailable(from, neg(sum(value, fee))); if (tx.getGas() > config.vmMaxBlockGasLimit()) { result.setCode(Code.INVALID_GAS);