Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added check on veichle identification number uniqueness #4

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 23 additions & 21 deletions src/main/java/io/horizen/lambo/CarRegistryAppModule.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package io.horizen.lambo;

import com.google.inject.AbstractModule;
import com.google.inject.Provides;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
import com.google.inject.name.Names;
import com.horizen.SidechainSettings;
import com.horizen.api.http.ApplicationApiGroup;
Expand Down Expand Up @@ -93,12 +95,6 @@ protected void configure() {
SidechainTransactionsCompanion transactionsCompanion = new SidechainTransactionsCompanion(
customTransactionSerializers, sidechainBoxesDataCompanion, sidechainProofsCompanion);


// Define Application state and wallet logic:
ApplicationWallet defaultApplicationWallet = new CarRegistryApplicationWallet();
ApplicationState defaultApplicationState = new CarRegistryApplicationState();


// Define the path to storages:
String dataDirPath = sidechainSettings.scorexSettings().dataDir().getAbsolutePath();
File secretStore = new File( dataDirPath + "/secret");
Expand All @@ -108,18 +104,11 @@ protected void configure() {
File stateStore = new File(dataDirPath + "/state");
File historyStore = new File(dataDirPath + "/history");
File consensusStore = new File(dataDirPath + "/consensusData");


// Add car registry specific API endpoints:
// CarApi endpoints processing will be added to the API server.
List<ApplicationApiGroup> customApiGroups = new ArrayList<>();
customApiGroups.add(new CarApi(transactionsCompanion));

File carInfoStore = new File(dataDirPath + "/cars");

// No core API endpoints to be disabled:
List<Pair<String, String>> rejectedApiPaths = new ArrayList<>();


// Inject custom objects:
// Names are equal to the ones specified in SidechainApp class constructor.
bind(SidechainSettings.class)
Expand All @@ -142,13 +131,13 @@ protected void configure() {
.annotatedWith(Names.named("CustomTransactionSerializers"))
.toInstance(customTransactionSerializers);

// Define Application state and wallet logic:
bind(ApplicationWallet.class)
.annotatedWith(Names.named("ApplicationWallet"))
.toInstance(defaultApplicationWallet);

.to(CarRegistryApplicationWallet.class);
bind(ApplicationState.class)
.annotatedWith(Names.named("ApplicationState"))
.toInstance(defaultApplicationState);
.to(CarRegistryApplicationState.class);

bind(Storage.class)
.annotatedWith(Names.named("SecretStorage"))
Expand All @@ -171,13 +160,26 @@ protected void configure() {
bind(Storage.class)
.annotatedWith(Names.named("ConsensusStorage"))
.toInstance(IODBStorageUtil.getStorage(consensusStore));

bind(new TypeLiteral<List<ApplicationApiGroup>> () {})
.annotatedWith(Names.named("CustomApiGroups"))
.toInstance(customApiGroups);
bind(Storage.class)
.annotatedWith(Names.named("CarInfoStorage"))
.toInstance(IODBStorageUtil.getStorage(carInfoStore));

bind(new TypeLiteral<List<Pair<String, String>>> () {})
.annotatedWith(Names.named("RejectedApiPaths"))
.toInstance(rejectedApiPaths);

bind(SidechainTransactionsCompanion.class)
.annotatedWith(Names.named("SidechainTransactionsCompanion"))
.toInstance(transactionsCompanion);
}

// Add car registry specific API endpoints:
// CarApi endpoints processing will be added to the API server.
@Provides @Named("CustomApiGroups")
List<ApplicationApiGroup> getCustomApiGroups(CarApi carApi) {
List<ApplicationApiGroup> customApiGroups = new ArrayList<>();
customApiGroups.add(carApi);
return customApiGroups;
}

}
62 changes: 54 additions & 8 deletions src/main/java/io/horizen/lambo/CarRegistryApplicationState.java
Original file line number Diff line number Diff line change
@@ -1,39 +1,85 @@
package io.horizen.lambo;

import com.google.inject.Inject;
import com.horizen.block.SidechainBlock;
import com.horizen.box.Box;
import com.horizen.proposition.Proposition;
import com.horizen.state.ApplicationState;
import com.horizen.state.SidechainStateReader;
import com.horizen.transaction.BoxTransaction;
import io.horizen.lambo.car.box.CarBox;
import io.horizen.lambo.car.box.CarSellOrderBox;
import io.horizen.lambo.car.services.CarInfoDBService;
import io.horizen.lambo.car.transaction.CarDeclarationTransaction;
import scala.util.Success;
import scala.util.Try;

import java.util.HashSet;
import java.util.List;
import java.util.Set;


// There is no custom logic for Car registry State now.
// TODO: prevent the declaration of CarBoxes which car information already exists in the previously added CarBoxes or CarSellOrderBoxes.
public class CarRegistryApplicationState implements ApplicationState {

private CarInfoDBService carInfoDbService;

@Inject
public CarRegistryApplicationState(CarInfoDBService carInfoDbService) {
this.carInfoDbService = carInfoDbService;
}

@Override
public boolean validate(SidechainStateReader stateReader, SidechainBlock block) {
return true;
return true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

@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())){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not if(transaction instanceof CarDeclarationTransaction) ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got it work by adding explicitly this generic signature to CarDeclarationTransaction
CarDeclarationTransaction<P extends Proposition, B extends NoncedBox

>

Set<String> vinList = carInfoDbService.extractVinFromBoxes(transaction.newBoxes());
for (String vin : vinList) {
if (! carInfoDbService.validateVin(vin, null)){
return false;
}
}
}
return true;
}

@Override
public Try<ApplicationState> onApplyChanges(SidechainStateReader stateReader, byte[] version, List<Box<Proposition>> newBoxes, List<byte[]> boxIdsToRemove) {
// TODO: here we expect to update Car info database. The data from it will be used during validation.
public Try<ApplicationState> onApplyChanges(SidechainStateReader stateReader,
byte[] version,
List<Box<Proposition>> newBoxes, List<byte[]> boxIdsToRemove) {
//we update the Car info database. The data from it will be used during validation.

//collect the vin to be added: the ones declared in new boxes
Set<String> vinToAdd = carInfoDbService.extractVinFromBoxes(newBoxes);
//collect the vin to be removed: the ones contained in the removed boxes that are not present in the prevoius list
Set<String> vinToRemove = new HashSet<>();
for (byte[] boxId : boxIdsToRemove) {
stateReader.getClosedBox(boxId).ifPresent( box -> {
if (box instanceof CarBox){
String vin = ((CarBox)box).getVin();
if (!vinToAdd.contains(vin)){
vinToRemove.add(vin);
}
} else if (box instanceof CarSellOrderBox){
String vin = ((CarSellOrderBox)box).getVin();
if (!vinToAdd.contains(vin)){
vinToRemove.add(vin);
}
}
}
);
}
carInfoDbService.updateVin(version, vinToAdd, vinToRemove);
return new Success<>(this);
}


@Override
public Try<ApplicationState> onRollback(byte[] version) {
// TODO: rollback car info database to certain point.
carInfoDbService.rollback(version);
return new Success<>(this);
}
}
13 changes: 12 additions & 1 deletion src/main/java/io/horizen/lambo/car/api/CarApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import akka.http.javadsl.server.Route;
import com.fasterxml.jackson.annotation.JsonView;
import com.google.inject.Inject;
import com.google.inject.name.Named;
import com.horizen.api.http.ApiResponse;
import com.horizen.api.http.ApplicationApiGroup;
import com.horizen.api.http.ErrorResponse;
Expand Down Expand Up @@ -30,6 +32,7 @@
import io.horizen.lambo.car.info.CarBuyOrderInfo;
import io.horizen.lambo.car.info.CarSellOrderInfo;
import io.horizen.lambo.car.proof.SellOrderSpendingProof;
import io.horizen.lambo.car.services.CarInfoDBService;
import io.horizen.lambo.car.transaction.BuyCarTransaction;
import io.horizen.lambo.car.transaction.CarDeclarationTransaction;
import io.horizen.lambo.car.transaction.SellCarTransaction;
Expand All @@ -54,9 +57,12 @@
public class CarApi extends ApplicationApiGroup {

private final SidechainTransactionsCompanion sidechainTransactionsCompanion;
private CarInfoDBService carInfoDBService;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS: in the SDK we are going to add a possibility to get ApplicationState and ApplicationWallet instances from the SidechainNodeView

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok! I add the issue


public CarApi(SidechainTransactionsCompanion sidechainTransactionsCompanion) {
@Inject
public CarApi(@Named("SidechainTransactionsCompanion") SidechainTransactionsCompanion sidechainTransactionsCompanion, CarInfoDBService carInfoDBService) {
this.sidechainTransactionsCompanion = sidechainTransactionsCompanion;
this.carInfoDBService = carInfoDBService;
}

// Define the base path for API url, i.e. according current config we could access that Api Group by using address 127.0.0.1:9085/carApi
Expand Down Expand Up @@ -92,6 +98,11 @@ private ApiResponse createCar(SidechainNodeView view, CreateCarBoxRequest ent) {
PublicKey25519Proposition carOwnershipProposition = PublicKey25519PropositionSerializer.getSerializer()
.parseBytes(BytesUtils.fromHexString(ent.proposition));

//check that the vin is unique (both in local veichle store and in mempool)
if (! carInfoDBService.validateVin(ent.vin, view.getNodeMemoryPool())){
throw new IllegalStateException("Vehicle identification number already present in blockchain");
}

CarBoxData carBoxData = new CarBoxData(carOwnershipProposition, ent.vin, ent.year, ent.model, ent.color);

// Try to collect regular boxes to pay fee
Expand Down
110 changes: 110 additions & 0 deletions src/main/java/io/horizen/lambo/car/services/CarInfoDBService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
package io.horizen.lambo.car.services;

import com.google.inject.Inject;
import com.google.inject.name.Named;
import com.horizen.box.Box;
import com.horizen.node.NodeMemoryPool;
import com.horizen.proposition.Proposition;
import com.horizen.storage.Storage;
import com.horizen.transaction.BoxTransaction;
import com.horizen.utils.ByteArrayWrapper;
import com.horizen.utils.Pair;
import io.horizen.lambo.car.box.CarBox;
import io.horizen.lambo.car.box.CarSellOrderBox;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.nio.ByteBuffer;
import java.util.Set;
import java.util.HashSet;
import java.util.List;
import java.util.ArrayList;

/**
* This service manages a local db with the list of all veichle identification numbers (vin) declared on the chain.
* The vin could be present inside two type of boxes: CarBox and CarSellOrderBox.
*/
public class CarInfoDBService {

private Storage carInfoStorage;
protected Logger log = LoggerFactory.getLogger(CarInfoDBService.class.getName());

@Inject
public CarInfoDBService(@Named("CarInfoStorage") Storage carInfoStorage){
this.carInfoStorage = carInfoStorage;
}

public void updateVin(byte[] version, Set<String> vinToAdd, Set<String> vinToRemove){
log.debug("carInfoStorage updateVin");
log.debug(" vinToAdd "+vinToAdd.size());
log.debug(" vinToRemove "+vinToRemove.size());
List<Pair<ByteArrayWrapper, ByteArrayWrapper>> toUpdate = new ArrayList<>(vinToAdd.size());
List<ByteArrayWrapper> toRemove = new ArrayList<>(vinToRemove.size());
vinToAdd.forEach(ele -> {
toUpdate.add(buildDBElement(ele));
});
vinToRemove.forEach(ele -> {
toRemove.add(buildDBElement(ele).getKey());
});
carInfoStorage.update(new ByteArrayWrapper(version), toUpdate, toRemove);
log.debug("carInfoStorage now contains: "+carInfoStorage.getAll().size()+" elements");
}


/**
* Validate the given vehicle identification number against the db list and (optionally) the mempool transactions.
* @param vin the vehicle identification number to check
* @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){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (carInfoStorage.get(buildDBElement(vin).getKey()).isPresent()){
return false;
}
//in the vin is not found, and the mempool was provided, we check also there
if (memoryPool != null) {
for (BoxTransaction<Proposition, Box<Proposition>> transaction : memoryPool.getTransactions()) {
Set<String> vinInMempool = extractVinFromBoxes(transaction.newBoxes());
if (vinInMempool.contains(vin)){
return false;
}
}
}
//if we arrive here, the vin is valid
return true;
}

public void rollback(byte[] version) {
carInfoStorage.rollback(new ByteArrayWrapper(version));
}

/**
* Extracts the list of vehicle identification numbers (vin) declared in the given box list.
* The vin could be present inside two type of boxes: CarBox and CarSellOrderBox
*/
public Set<String> extractVinFromBoxes(List<Box<Proposition>> boxes){
Set<String> vinList = new HashSet<String>();
for (Box<Proposition> currentBox : boxes) {
if (CarBox.class.isAssignableFrom(currentBox.getClass())){
String vin = CarBox.parseBytes(currentBox.bytes()).getVin();
vinList.add(vin);
} else if (CarSellOrderBox.class.isAssignableFrom(currentBox.getClass())){
String vin = CarSellOrderBox.parseBytes(currentBox.bytes()).getVin();
vinList.add(vin);
}
}
return vinList;
}



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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

ByteArrayWrapper keyWrapper = new ByteArrayWrapper(keyBuffer.array());
//the value is not important (we need just a key set, each key is a vin)
ByteArrayWrapper valueWrapper = new ByteArrayWrapper(new byte[1]);
return new Pair<>(keyWrapper, valueWrapper);
}

}