added check on veichle identification number uniqueness #4
Conversation
| @Override | ||
| public boolean validate(SidechainStateReader stateReader, SidechainBlock block) { | ||
| return true; | ||
| return true; |
There was a problem hiding this comment.
We should also check that there are no multiple transactions declaring the same VIN inside the block.
In the validate method below we only check the uniqueness against state.
| @@ -54,9 +57,12 @@ | |||
| public class CarApi extends ApplicationApiGroup { | |||
|
|
|||
| private final SidechainTransactionsCompanion sidechainTransactionsCompanion; | |||
| private CarInfoDBService carInfoDBService; | |||
There was a problem hiding this comment.
CarInfoDBService must be controlled only by CarRegistryApplicationState class. Other way it is possible to change the state of CarInfoDBService directly in CarApi.
I guess that it is done in this way, because there is no way to get access to the application state instance inside the CarApi, because there is no interface method inside the SidechainNodeView, right?
There was a problem hiding this comment.
PS: in the SDK we are going to add a possibility to get ApplicationState and ApplicationWallet instances from the SidechainNodeView
There was a problem hiding this comment.
yes the reason was that one!
wating for the method will be available in SDK I could otherwise inject ApplicationState in the constructor
what do you think?
There was a problem hiding this comment.
Let's keep it as is and post the issue to the lambo repo.
After the changes in the SDK we will change here as well.
Note: it's important, that SidechainNodeView should provide the access to the "read-only" part of the CarRegistryApplicationState, otherwise we will have the same issue as now.
| * @param memoryPool if not null, the vin is checked also against the mempool transactions | ||
| * @return true if the vin is valid (not already declared) | ||
| */ | ||
| public boolean validateVin(String vin, NodeMemoryPool memoryPool){ |
There was a problem hiding this comment.
Inside the SDK we try to use Option/Optional wrappers to avoid passing null arguments.
I would suggest keep the same style in Lambo, so to change the method like:
public boolean validateVin(String vin, Optional<NodeMemoryPool> memoryPoolOpt)
PS: not mandatory.
|
|
||
| private Pair<ByteArrayWrapper, ByteArrayWrapper> buildDBElement(String vin){ | ||
| //we use a fixed key size of 32, which is the default of iohk.iodb used as underline storage | ||
| ByteBuffer keyBuffer = ByteBuffer.allocate(32).put(vin.getBytes()); |
There was a problem hiding this comment.
Instead of truncating the part of the string (in case if for some reason its byte representation is longer than 32) just use Blake2b256.hash method.
| } | ||
|
|
||
| @Override | ||
| public boolean validate(SidechainStateReader stateReader, BoxTransaction<Proposition, Box<Proposition>> transaction) { | ||
| // TODO: here we expect to go though all CarDeclarationTransactions and verify that each CarBox reflects to unique Car. | ||
| // we go though all CarDeclarationTransactions and verify that each CarBox reflects to unique Car. | ||
| if (CarDeclarationTransaction.class.isAssignableFrom(transaction.getClass())){ |
There was a problem hiding this comment.
why not if(transaction instanceof CarDeclarationTransaction) ?
There was a problem hiding this comment.
I tried it but it gives this compilation error:
java: incompatible types: com.horizen.transaction.BoxTransaction<com.horizen.proposition.Proposition,com.horizen.box.Box<com.horizen.proposition.Proposition>> cannot be converted to io.horizen.lambo.car.transaction.CarDeclarationTransaction
(to be honest I don't understand what is the problem because the hierachy seems compatible!)
There was a problem hiding this comment.
I got it work by adding explicitly this generic signature to CarDeclarationTransaction
CarDeclarationTransaction<P extends Proposition, B extends NoncedBox
>
| @@ -27,10 +27,9 @@ | |||
| // As outputs it contains possible RegularBoxes(to pay fee and change) and new CarBox entry. | |||
| // No specific unlockers to parent class logic, but has specific new box. | |||
| // TODO: add specific mempool incompatibility checker to deprecate keeping in the Mempool txs that declare the same Car. | |||
| public final class CarDeclarationTransaction extends AbstractRegularTransaction { | |||
| public final class CarDeclarationTransaction<P extends Proposition, B extends NoncedBox<P>> extends AbstractRegularTransaction { | |||
There was a problem hiding this comment.
I don't think that it's correct to define generics here, because in one of the parent classes the proper values were already set to the templates. The previous error with instanceof occurs because of the generics, which are not covariant.
I would suggest to revert current line changes and in the error place use stuff like CarDeclarationTransaction.class.isInstance(t).
Should be fine.
No description provided.