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

Add no key hashing option to ProofMapIndexProxy [ECR-3779] #1222

Merged
merged 15 commits into from
Nov 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion exonum-java-binding/core/checkstyle-suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<!-- Do not require Javadoc for native adapters -->
<suppress files="service/adapters.+.java" checks="JavadocMethod"/>

<suppress files="ProofMapIndexProxyIntegrationTest\.java"
<suppress files="ProofMapIndexProxyNoKeyHashingIntegrationTest\.java"
checks="Javadoc.*"/>

<!-- Allow `aBlock` name -->
Expand Down
4 changes: 4 additions & 0 deletions exonum-java-binding/core/rust/src/storage/proof_map_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ pub extern "system" fn Java_com_exonum_binding_core_storage_indices_ProofMapInde
_: JClass,
name: JString,
view_handle: Handle,
// TODO: to be used in ECR-3765
_key_hashing: jboolean,
) -> Handle {
let res = panic::catch_unwind(|| {
let name = utils::convert_to_string(&env, name)?;
Expand All @@ -88,6 +90,8 @@ pub extern "system" fn Java_com_exonum_binding_core_storage_indices_ProofMapInde
group_name: JString,
map_id: jbyteArray,
view_handle: Handle,
// TODO: to be used in ECR-3765
_key_hashing: jboolean,
) -> Handle {
let res = panic::catch_unwind(|| {
let group_name = utils::convert_to_string(&env, group_name)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ public MapIndex<HashCode, TransactionMessage> getTxMessages() {
}

/**
* Returns a map with a key-value pair of a transaction hash and execution result.
* Returns a map with a key-value pair of a transaction hash and execution result. Note that this
* is a <a href="ProofMapIndexProxy.html#key-hashing">proof map that uses non-hashed keys</a>.
*/
public ProofMapIndexProxy<HashCode, ExecutionStatus> getTxResults() {
return schema.getTxResults();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,11 @@ MapIndex<HashCode, TransactionMessage> getTxMessages() {
}

/**
* Returns a map with a key-value pair of a transaction hash and execution result.
* Returns a map with a key-value pair of a transaction hash and execution result. Note that this
* is a <a href="ProofMapIndexProxy.html#key-hashing">proof map that uses non-hashed keys</a>.
*/
ProofMapIndexProxy<HashCode, ExecutionStatus> getTxResults() {
return ProofMapIndexProxy.newInstance(CoreIndex.TRANSACTIONS_RESULTS, dbView,
return ProofMapIndexProxy.newInstanceNoKeyHashing(CoreIndex.TRANSACTIONS_RESULTS, dbView,
StandardSerializers.hash(), EXECUTION_STATUS_SERIALIZER);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,22 +94,23 @@ default void putAll(Map<? extends K, ? extends V> sourceMap) {
void remove(K key);

/**
* Returns an iterator over the map keys in lexicographical order.
* Returns an iterator over the map keys. The keys are ordered in lexicographical order.
*
* @throws IllegalStateException if this map is not valid
*/
Iterator<K> keys();

/**
* Returns an iterator over the map values in lexicographical order of <em>keys</em>.
* Returns an iterator over the map values. The values are ordered in lexicographical order of
* <em>keys</em>.
*
* @throws IllegalStateException if this map is not valid
*/
Iterator<V> values();

/**
* Returns an iterator over the map entries.
* The entries are ordered by keys in lexicographical order.
* Returns an iterator over the map entries. The entries are ordered by keys in lexicographical
* order.
*
* @throws IllegalStateException if this map is not valid
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,29 @@
* that a certain key is mapped to a particular value <em>or</em> that there are no mapping for
* the key in the map.
*
* <p>This map is implemented as a Merkle-Patricia tree. It does not permit null keys and values,
* and requires that keys are 32-byte long.
* <p>This map is implemented as a Merkle-Patricia tree. It does not permit null keys and values.
*
* <p>The Merkle-Patricia tree backing the proof map uses internal 32-byte keys. The tree balance
* relies on the internal keys being uniformly distributed.
*
* <h3><a name="key-hashing">Key hashing in proof maps</a></h3>
*
* <p>By default, when creating the proof map using methods
* {@link #newInstance(String, View, Serializer, Serializer)} and
* {@link #newInGroupUnsafe(String, byte[], View, Serializer, Serializer)}, the user keys are
* converted into internal keys through hashing. This allows to use keys of arbitrary size and
* ensures the balance of the internal tree. It is also possible to create a proof map that will
* not hash keys with methods {@link #newInstanceNoKeyHashing(String, View, Serializer, Serializer)}
* and {@link #newInGroupUnsafeNoKeyHashing(String, byte[], View, Serializer, Serializer)}. In this
* mode, the map will use the user keys as internal tree keys. Such mode of operation is
* appropriate iff all of the following conditions hold:
*
* <ul>
* <li>All keys are 32-byte long</li>
* <li>The keys are uniformly distributed</li>
* <li>The keys come from a trusted source that cannot influence their distribution and affect
* the tree balance.</li>
* </ul>
*
* <p>The "destructive" methods of the map, i.e., the one that change the map contents,
* are specified to throw {@link UnsupportedOperationException} if
Expand All @@ -58,13 +79,13 @@
* <p>When the view goes out of scope, this map is destroyed. Subsequent use of the closed map
* is prohibited and will result in {@link IllegalStateException}.
*
* @param <K> the type of keys in this map. Must be 32-byte long in the serialized form
* @param <K> the type of keys in this map
* @param <V> the type of values in this map
* @see View
*/
public final class ProofMapIndexProxy<K, V> extends AbstractIndexProxy implements MapIndex<K, V> {

private final ProofMapKeyCheckingSerializerDecorator<K> keySerializer;
private final Serializer<K> keySerializer;
private final CheckingSerializerDecorator<V> valueSerializer;

/**
Expand All @@ -74,7 +95,7 @@ public final class ProofMapIndexProxy<K, V> extends AbstractIndexProxy implement
* [a-zA-Z0-9_]
* @param view a database view. Must be valid.
* If a view is read-only, "destructive" operations are not permitted.
* @param keySerializer a serializer of keys, must always produce 32-byte long values
* @param keySerializer a serializer of keys
* @param valueSerializer a serializer of values
* @param <K> the type of keys in the map
* @param <V> the type of values in the map
Expand All @@ -86,9 +107,34 @@ public static <K, V> ProofMapIndexProxy<K, V> newInstance(
String name, View view, Serializer<K> keySerializer, Serializer<V> valueSerializer) {
IndexAddress address = IndexAddress.valueOf(name);
long viewNativeHandle = view.getViewNativeHandle();
LongSupplier nativeMapConstructor = () -> nativeCreate(name, viewNativeHandle);
LongSupplier nativeMapConstructor = () -> nativeCreate(name, viewNativeHandle, true);

return getOrCreate(address, view, keySerializer, valueSerializer, nativeMapConstructor, true);
}

/**
* Creates a <a href="ProofMapIndexProxy.html#key-hashing">ProofMapIndexProxy that uses non-hashed keys</a>.
* Requires that keys are 32-byte long.
*
* @param name a unique alphanumeric non-empty identifier of this map in the underlying storage:
* [a-zA-Z0-9_]
* @param view a database view. Must be valid.
* If a view is read-only, "destructive" operations are not permitted.
* @param keySerializer a serializer of keys, must always produce 32-byte long values
* @param valueSerializer a serializer of values
* @param <K> the type of keys in the map
* @param <V> the type of values in the map
* @throws IllegalStateException if the view is not valid
* @throws IllegalArgumentException if the name is empty
* @see StandardSerializers
*/
public static <K, V> ProofMapIndexProxy<K, V> newInstanceNoKeyHashing(
String name, View view, Serializer<K> keySerializer, Serializer<V> valueSerializer) {
IndexAddress address = IndexAddress.valueOf(name);
long viewNativeHandle = view.getViewNativeHandle();
LongSupplier nativeMapConstructor = () -> nativeCreate(name, viewNativeHandle, false);

return getOrCreate(address, view, keySerializer, valueSerializer, nativeMapConstructor);
return getOrCreate(address, view, keySerializer, valueSerializer, nativeMapConstructor, false);
}

/**
Expand All @@ -100,6 +146,36 @@ public static <K, V> ProofMapIndexProxy<K, V> newInstance(
* @param groupName a name of the collection group
* @param mapId an identifier of this collection in the group, see the caveats
* @param view a database view
* @param keySerializer a serializer of keys
* @param valueSerializer a serializer of values
* @param <K> the type of keys in the map
* @param <V> the type of values in the map
* @return a new map proxy
* @throws IllegalStateException if the view is not valid
* @throws IllegalArgumentException if the name or index id is empty
* @see StandardSerializers
*/
public static <K, V> ProofMapIndexProxy<K, V> newInGroupUnsafe(
String groupName, byte[] mapId, View view, Serializer<K> keySerializer,
Serializer<V> valueSerializer) {
IndexAddress address = IndexAddress.valueOf(groupName, mapId);
long viewNativeHandle = view.getViewNativeHandle();
LongSupplier nativeMapConstructor =
() -> nativeCreateInGroup(groupName, mapId, viewNativeHandle, true);

return getOrCreate(address, view, keySerializer, valueSerializer, nativeMapConstructor, true);
}

/**
* Creates a new <a href="ProofMapIndexProxy.html#key-hashing">proof map that uses non-hashed keys</a>
* in a <a href="package-summary.html#families">collection group</a> with the given name.
* Requires that keys are 32-byte long.
*
* <p>See a <a href="package-summary.html#families-limitations">caveat</a> on index identifiers.
*
* @param groupName a name of the collection group
* @param mapId an identifier of this collection in the group, see the caveats
* @param view a database view
* @param keySerializer a serializer of keys, must always produce 32-byte long values
* @param valueSerializer a serializer of values
* @param <K> the type of keys in the map
Expand All @@ -109,26 +185,24 @@ public static <K, V> ProofMapIndexProxy<K, V> newInstance(
* @throws IllegalArgumentException if the name or index id is empty
* @see StandardSerializers
*/
public static <K, V> ProofMapIndexProxy<K, V> newInGroupUnsafe(String groupName,
byte[] mapId,
View view,
Serializer<K> keySerializer,
Serializer<V> valueSerializer) {
public static <K, V> ProofMapIndexProxy<K, V> newInGroupUnsafeNoKeyHashing(
String groupName, byte[] mapId, View view, Serializer<K> keySerializer,
Serializer<V> valueSerializer) {
IndexAddress address = IndexAddress.valueOf(groupName, mapId);
long viewNativeHandle = view.getViewNativeHandle();
LongSupplier nativeMapConstructor =
() -> nativeCreateInGroup(groupName, mapId, viewNativeHandle);
() -> nativeCreateInGroup(groupName, mapId, viewNativeHandle, false);

return getOrCreate(address, view, keySerializer, valueSerializer, nativeMapConstructor);
return getOrCreate(address, view, keySerializer, valueSerializer, nativeMapConstructor, false);
}

private static <K, V> ProofMapIndexProxy<K, V> getOrCreate(IndexAddress address, View view,
Serializer<K> keySerializer, Serializer<V> valueSerializer,
LongSupplier nativeMapConstructor) {
LongSupplier nativeMapConstructor, boolean keyHashing) {
return view.findOpenIndex(address)
.map(ProofMapIndexProxy::<K, V>checkCachedInstance)
.orElseGet(() -> newMapIndexProxy(address, view, keySerializer, valueSerializer,
nativeMapConstructor));
nativeMapConstructor, keyHashing));
}

@SuppressWarnings("unchecked") // The compiler is correct: the cache is not type-safe: ECR-3387
Expand All @@ -139,9 +213,8 @@ private static <K, V> ProofMapIndexProxy<K, V> checkCachedInstance(StorageIndex

private static <K, V> ProofMapIndexProxy<K, V> newMapIndexProxy(IndexAddress address, View view,
Serializer<K> keySerializer, Serializer<V> valueSerializer,
LongSupplier nativeMapConstructor) {
ProofMapKeyCheckingSerializerDecorator<K> ks =
ProofMapKeyCheckingSerializerDecorator.from(keySerializer);
LongSupplier nativeMapConstructor, boolean keyHashing) {
Serializer<K> ks = decorateKeySerializer(keySerializer, keyHashing);
CheckingSerializerDecorator<V> vs = CheckingSerializerDecorator.from(valueSerializer);

NativeHandle mapNativeHandle = createNativeMap(view, nativeMapConstructor);
Expand All @@ -151,6 +224,15 @@ private static <K, V> ProofMapIndexProxy<K, V> newMapIndexProxy(IndexAddress add
return map;
}

private static <K> Serializer<K> decorateKeySerializer(
Serializer<K> keySerializer, boolean keyHashing) {
if (!keyHashing) {
return ProofMapKeyCheckingSerializerDecorator.from(keySerializer);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed at all if it can return just keySerializer, with no extra decorator types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To check for not null values?

Copy link
Contributor

Choose a reason for hiding this comment

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

There is one for that already.

Copy link
Contributor

Choose a reason for hiding this comment

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

com.exonum.binding.common.serialization.CheckingSerializerDecorator

} else {
return CheckingSerializerDecorator.from(keySerializer);
}
}

private static NativeHandle createNativeMap(View view, LongSupplier nativeMapConstructor) {
NativeHandle mapNativeHandle = new NativeHandle(nativeMapConstructor.getAsLong());

Expand All @@ -160,13 +242,13 @@ private static NativeHandle createNativeMap(View view, LongSupplier nativeMapCon
return mapNativeHandle;
}

private static native long nativeCreate(String name, long viewNativeHandle);
private static native long nativeCreate(String name, long viewNativeHandle, boolean keyHashing);

private static native long nativeCreateInGroup(String groupName, byte[] mapId,
long viewNativeHandle);
long viewNativeHandle, boolean keyHashing);

private ProofMapIndexProxy(NativeHandle nativeHandle, IndexAddress address, View view,
ProofMapKeyCheckingSerializerDecorator<K> keySerializer,
Serializer<K> keySerializer,
CheckingSerializerDecorator<V> valueSerializer) {
super(nativeHandle, address, view);
this.keySerializer = keySerializer;
Expand All @@ -184,10 +266,11 @@ public boolean containsKey(K key) {
/**
* {@inheritDoc}
*
* @param key a proof map key, must be 32-byte long when serialized
* @param key a proof map key
* @param value a storage value to associate with the key
* @throws IllegalStateException if this map is not valid
* @throws IllegalArgumentException if the size of the key is not 32 bytes
* @throws IllegalArgumentException if the size of the key is not 32 bytes (in case of a
* <a href="ProofMapIndexProxy.html#key-hashing">proof map that uses non-hashed keys</a>)
* @throws UnsupportedOperationException if this map is read-only
*/
@Override
Expand Down Expand Up @@ -227,11 +310,11 @@ public V get(K key) {
* Returns a proof that there are values mapped to the specified keys or that there are no such
* mappings.
*
* @param key a proof map key which might be mapped to some value, must be 32-byte long
* @param otherKeys other proof map keys which might be mapped to some values, each must be
* 32-byte long
* @param key a proof map key which might be mapped to some value
* @param otherKeys other proof map keys which might be mapped to some values
* @throws IllegalStateException if this map is not valid
* @throws IllegalArgumentException if the size of any of the keys is not 32 bytes
* @throws IllegalArgumentException if the size of any of the keys is not 32 bytes (in case of a
* <a href="ProofMapIndexProxy.html#key-hashing">proof map that uses non-hashed keys</a>)
*/
public UncheckedMapProof getProof(K key, K... otherKeys) {
if (otherKeys.length == 0) {
Expand All @@ -246,10 +329,11 @@ public UncheckedMapProof getProof(K key, K... otherKeys) {
* Returns a proof that there are values mapped to the specified keys or that there are no such
* mappings.
*
* @param keys proof map keys which might be mapped to some values, each must be 32-byte long
* @param keys proof map keys which might be mapped to some values
* @throws IllegalStateException if this map is not valid
* @throws IllegalArgumentException if the size of any of the keys is not 32 bytes
* or keys collection is empty
* @throws IllegalArgumentException if the size of any of the keys is not 32 bytes (in case of a
* <a href="ProofMapIndexProxy.html#key-hashing">proof map that uses non-hashed keys</a>) or
* keys collection is empty
*/
public UncheckedMapProof getProof(Collection<? extends K> keys) {
checkArgument(!keys.isEmpty(), "Keys collection should not be empty");
Expand Down
Loading