Migrate service runtime and services to transaction methods [ECR-3994] - #1307
Conversation
|
|
||
| List<HistoryEntity> getWalletHistory(PublicKey ownerKey); | ||
|
|
||
| void createWalletTx(TxMessageProtos.CreateWalletTx arguments, TransactionContext context) |
There was a problem hiding this comment.
(Here and in similar cases)
I'd drop "Tx" suffix (just createWallet — a service operation).
| new CryptocurrencySchema(context.getFork(), context.getServiceName()); | ||
| MapIndex<PublicKey, Wallet> wallets = schema.wallets(); | ||
|
|
||
| if (wallets.containsKey(ownerPublicKey)) { |
There was a problem hiding this comment.
May also use checkExecution(!wallets.containsKey(ownerPublicKey), WALLET_ALREADY_EXISTS.errorCode), just as the next method.
|
|
||
| private static final Logger logger = LogManager.getLogger(QaService.class); | ||
|
|
||
| private static int CREATE_COUNTER_TX_ID = 0; |
There was a problem hiding this comment.
The constants duplicate the values in QaTransaction enum — shall probably re-use (or one of them shall be deleted)?
There was a problem hiding this comment.
Yes, was going to delete the enum
| package com.exonum.binding.cryptocurrency.transactions; | ||
|
|
||
| enum TransactionError { | ||
| public enum TransactionError { |
There was a problem hiding this comment.
It's the last remaining class in this package — may move to its parent package and keep package-private.
| -1, | ||
| 0 | ||
| }) | ||
| void fromRawTransactionRejectsNonPositiveBalance(long transferAmount) { |
There was a problem hiding this comment.
As the condition was moved from the constructor to execute, I'd keep the test as executeTransferNegativeBalance
|
The trend with removing code (and keeping functionality) in recent PRs is very good 🙃 :
|
It temporarily has to use 'runtime' error code.
The remaining are caused by some commented out code.
Inject TransactionInvoker in ServiceWrapper.
Verify that they indeed clear the schema.
Check the max error code before casting Restore proper tests
Also, report it as an execution exception
The constants used in annotations _must_ be compile-time constants: values obtained through QaTransaction#id call are not, hence cannot be used there.
|
b01ad54 passes all tests locally. I specifically didn't merged @vitvakatu, @bondar If you don't feel like reviewing the whole thing, please review 2687530...b01ad54 with various fixes and improvements. The remaining thing — is |
Overview
Migrate service runtime and services to transaction methods.
See: https://jira.bf.local/browse/ECR-3994
Definition of Done