Skip to content

Commit

Permalink
Address PR comments.
Browse files Browse the repository at this point in the history
Signed-off-by: Sergiy Yevtushenko <[email protected]>
Signed-off-by: Sergiy Yevtushenko <[email protected]>
  • Loading branch information
siy committed Sep 9, 2024
1 parent d49e9b4 commit a2829da
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 71 deletions.
4 changes: 2 additions & 2 deletions core-rust/p2p/src/address_book_components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ use crate::engine_prelude::{ScryptoCategorize, ScryptoDecode, ScryptoEncode};
use sbor::define_single_versioned;

/// Timestamp of the various peer-related events
// At present it's just an alias for i64. Later we may want to replace it with struct using crono crate and
// At present, it's just an alias for i64. Later we may want to replace it with struct using crono crate and
// do something like shown below to transparently convert to/from internal representation
// (once there will be real usage at Rust side).
// #[sbor(
Expand All @@ -94,7 +94,7 @@ pub enum ConnectionStatus {

#[derive(Debug, Clone, ScryptoCategorize, ScryptoEncode, ScryptoDecode)]
pub struct NodeIdDTO {
pub key: components::RawPublicKey,
pub key: components::NodeSecp256k1PublicKey,
}

/// Address book entry
Expand Down
4 changes: 2 additions & 2 deletions core-rust/p2p/src/column_families.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
*/

use crate::address_book_components::*;
use crate::components::RawPublicKey;
use crate::components::NodeSecp256k1PublicKey;
use crate::migration::MigrationStatus;
use crate::safety_store_components::{SafetyState, VersionedSafetyState};
use crate::typed_cf_api::RawPublicKeyDbCodec;
Expand All @@ -83,7 +83,7 @@ impl DefaultCf for MigrationStatusCf {
/// Address book
pub struct AddressBookCf;
impl VersionedCf for AddressBookCf {
type Key = RawPublicKey;
type Key = NodeSecp256k1PublicKey;
type Value = AddressBookEntry;

const VERSIONED_NAME: &'static str = "address_book";
Expand Down
8 changes: 4 additions & 4 deletions core-rust/p2p/src/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use crate::engine_prelude::{ScryptoCategorize, ScryptoDecode, ScryptoEncode};
Debug, Clone, ScryptoCategorize, ScryptoEncode, ScryptoDecode, PartialOrd, Ord, PartialEq, Eq,
)]
#[sbor(transparent)]
pub struct RawPublicKey(pub [u8; RawPublicKey::LENGTH]);
pub struct NodeSecp256k1PublicKey(pub [u8; NodeSecp256k1PublicKey::LENGTH]);

impl RawPublicKey {
impl NodeSecp256k1PublicKey {
pub const LENGTH: usize = 33;

pub fn new(id: [u8; Self::LENGTH]) -> Self {
Expand All @@ -21,9 +21,9 @@ impl RawPublicKey {

/// The Secp256K1 signature
#[derive(Debug, Clone, ScryptoCategorize, ScryptoEncode, ScryptoDecode)]
pub struct Signature(pub [u8; Signature::LENGTH]);
pub struct NodeSignature(pub [u8; NodeSignature::LENGTH]);

impl Signature {
impl NodeSignature {
pub const LENGTH: usize = 65; // v(1) + r(32) + s(32)

pub fn new(signature: [u8; Self::LENGTH]) -> Self {
Expand Down
6 changes: 3 additions & 3 deletions core-rust/p2p/src/rocks_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ use node_common::rocksdb::{ColumnFamilyDescriptor, Options, DB};

use crate::address_book_components::*;
use crate::column_families::*;
use crate::components::RawPublicKey;
use crate::components::NodeSecp256k1PublicKey;
use crate::engine_prelude::*;
use crate::migration::MigrationStatus;
use crate::safety_store_components::SafetyState;
Expand Down Expand Up @@ -157,7 +157,7 @@ impl ActualSafetyStoreDatabase {
}

impl<R: WriteableRocks> AddressBookStore for AddressBookDatabase<R> {
fn remove_one(&self, node_id: &RawPublicKey) -> bool {
fn remove_one(&self, node_id: &NodeSecp256k1PublicKey) -> bool {
let binding = open_rw_context(&self.rocks);
let context = binding.cf(AddressBookCf);

Expand All @@ -168,7 +168,7 @@ impl<R: WriteableRocks> AddressBookStore for AddressBookDatabase<R> {
true
}

fn upsert_one(&self, node_id: &RawPublicKey, entry: &AddressBookEntry) -> bool {
fn upsert_one(&self, node_id: &NodeSecp256k1PublicKey, entry: &AddressBookEntry) -> bool {
let binding = open_rw_context(&self.rocks);
let context = binding.cf(AddressBookCf);

Expand Down
13 changes: 8 additions & 5 deletions core-rust/p2p/src/safety_store_components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
* permissions under this License.
*/

use crate::components::{RawPublicKey, Signature};
use crate::components::{NodeSecp256k1PublicKey, NodeSignature};
use crate::engine_prelude::*;
use sbor::define_single_versioned;

Expand Down Expand Up @@ -111,7 +111,7 @@ pub struct BFTValidator {
Debug, Clone, ScryptoCategorize, ScryptoEncode, ScryptoDecode, Ord, PartialOrd, Eq, PartialEq,
)]
pub struct BFTValidatorId {
key: RawPublicKey,
key: NodeSecp256k1PublicKey,
validator_address: ComponentAddress,
}

Expand All @@ -122,6 +122,9 @@ pub struct HighQC {
highest_timeout_certificate: Option<TimeoutCertificate>,
}

// FIXME: A duplicate of LedgerHeader from StateManager.
// Made separate te reference only types within this module. De-duplication requires
// careful merging of the other referenced types as well.
#[derive(Debug, Clone, ScryptoCategorize, ScryptoEncode, ScryptoDecode)]
pub struct LedgerHeader {
epoch: i64,
Expand Down Expand Up @@ -172,7 +175,7 @@ pub struct TimeoutCertificate {
#[derive(Debug, Clone, ScryptoCategorize, ScryptoEncode, ScryptoDecode)]
pub struct TimestampedECDSASignature {
timestamp: SafetyStateTimestamp,
signature: Signature,
signature: NodeSignature,
}

#[derive(Debug, Clone, ScryptoCategorize, ScryptoEncode, ScryptoDecode)]
Expand All @@ -191,8 +194,8 @@ pub struct Vote {
high_quorum_certificate: HighQC,
vote_data: VoteData,
timestamp: SafetyStateTimestamp,
signature: Signature,
timeout_signature: Option<Signature>,
signature: NodeSignature,
timeout_signature: Option<NodeSignature>,
}

#[derive(Debug, Clone, ScryptoCategorize, ScryptoEncode, ScryptoDecode)]
Expand Down
6 changes: 3 additions & 3 deletions core-rust/p2p/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,12 @@ use crate::engine_prelude::*;
pub mod node {
use super::*;
use crate::address_book_components::*;
use crate::components::RawPublicKey;
use crate::components::NodeSecp256k1PublicKey;
use crate::safety_store_components::SafetyState;

pub trait AddressBookStore {
fn remove_one(&self, node_id: &RawPublicKey) -> bool;
fn upsert_one(&self, node_id: &RawPublicKey, entry: &AddressBookEntry) -> bool;
fn remove_one(&self, node_id: &NodeSecp256k1PublicKey) -> bool;
fn upsert_one(&self, node_id: &NodeSecp256k1PublicKey, entry: &AddressBookEntry) -> bool;
fn reset(&self);
fn get_all(&self) -> Vec<AddressBookEntry>;
fn is_migrated(&self) -> bool;
Expand Down
12 changes: 6 additions & 6 deletions core-rust/p2p/src/typed_cf_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,24 +62,24 @@
* permissions under this License.
*/

use crate::components::RawPublicKey;
use crate::components::NodeSecp256k1PublicKey;
use node_common::store::typed_cf_api::{BoundedDbCodec, DbCodec};

#[derive(Clone, Default)]
pub struct RawPublicKeyDbCodec {}

impl DbCodec<RawPublicKey> for RawPublicKeyDbCodec {
fn encode(&self, value: &RawPublicKey) -> Vec<u8> {
impl DbCodec<NodeSecp256k1PublicKey> for RawPublicKeyDbCodec {
fn encode(&self, value: &NodeSecp256k1PublicKey) -> Vec<u8> {
value.as_bytes().to_vec()
}

fn decode(&self, bytes: &[u8]) -> RawPublicKey {
RawPublicKey::new(bytes.try_into().expect("Invalid NodeId"))
fn decode(&self, bytes: &[u8]) -> NodeSecp256k1PublicKey {
NodeSecp256k1PublicKey::new(bytes.try_into().expect("Invalid NodeId"))
}
}

impl BoundedDbCodec for RawPublicKeyDbCodec {
fn upper_bound_encoding(&self) -> Vec<u8> {
vec![0xFF; RawPublicKey::LENGTH]
vec![0xFF; NodeSecp256k1PublicKey::LENGTH]
}
}
6 changes: 1 addition & 5 deletions core-rust/state-manager/src/jni/node_rust_environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,7 @@ impl JNINodeRustEnvironment {
}

fn combine(base: &String, ext: &str) -> PathBuf {
let mut path = base.clone();
path.push(MAIN_SEPARATOR);
path.push_str(ext);

PathBuf::from(path)
[base, ext].iter().collect()
}

pub fn cleanup(env: &JNIEnv, j_node_rust_env: JObject) {
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/com/radixdlt/RadixNodeModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,10 @@ protected void configure() {
protocolConfig,
ScenariosExecutionConfig.resolveForNetwork(network)));

// Persistence
install(new PersistentSafetyStateStoreModule());
install(new AddressBookModule());

// Recovery
install(new EpochsSafetyRecoveryModule());
install(new REv2LedgerRecoveryModule());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ public final class REv2StateManagerModule extends AbstractModule {
private final ProtocolConfig protocolConfig;
private final boolean noFees;
private final ScenariosExecutionConfig scenariosExecutionConfig;
private final boolean installPersistence;

private REv2StateManagerModule(
ProposalLimitsConfig proposalLimitsConfig,
Expand All @@ -128,8 +127,7 @@ private REv2StateManagerModule(
LedgerSyncLimitsConfig ledgerSyncLimitsConfig,
ProtocolConfig protocolConfig,
boolean noFees,
ScenariosExecutionConfig scenariosExecutionConfig,
boolean installPersistence) {
ScenariosExecutionConfig scenariosExecutionConfig) {
this.proposalLimitsConfig = proposalLimitsConfig;
this.vertexLimitsConfigOpt = vertexLimitsConfigOpt;
this.databaseConfig = databaseConfig;
Expand All @@ -141,7 +139,6 @@ private REv2StateManagerModule(
this.protocolConfig = protocolConfig;
this.noFees = noFees;
this.scenariosExecutionConfig = scenariosExecutionConfig;
this.installPersistence = installPersistence;
}

public static REv2StateManagerModule create(
Expand All @@ -165,8 +162,7 @@ public static REv2StateManagerModule create(
ledgerSyncLimitsConfig,
protocolConfig,
false,
scenariosExecutionConfig,
true);
scenariosExecutionConfig);
}

public static REv2StateManagerModule createForTesting(
Expand All @@ -191,8 +187,7 @@ public static REv2StateManagerModule createForTesting(
ledgerSyncLimitsConfig,
protocolConfig,
noFees,
scenariosExecutionConfig,
false);
scenariosExecutionConfig);
}

@Override
Expand Down Expand Up @@ -318,12 +313,6 @@ EventProcessor<BFTInsertUpdate> onInsertUpdatePersistVertexStore(
bind(new Key<MempoolInserter<RawNotarizedTransaction>>() {}).to(RustMempool.class);
bind(MempoolReevaluator.class).to(RustMempool.class);
}

if (installPersistence) {
// Moved here for convenience performing migration
install(new PersistentSafetyStateStoreModule());
install(new AddressBookModule());
}
}

@ProvidesIntoSet
Expand Down
33 changes: 6 additions & 27 deletions core/src/test/java/com/radixdlt/p2p/test/P2PTestNetworkRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@
import com.radixdlt.serialization.Serialization;
import com.radixdlt.statecomputer.ProtocolState;
import com.radixdlt.store.BerkeleyDbDefaults;
import com.radixdlt.store.NodeStorageLocation;
import com.radixdlt.utils.properties.RuntimeProperties;
import com.sleepycat.je.EnvironmentConfig;
import java.io.IOException;
import java.util.Objects;
import java.util.function.Function;
Expand Down Expand Up @@ -203,37 +203,16 @@ private static Injector createInjector(
protected void configure() {
bind(AddressBookPersistence.class).to(BerkeleyAddressBookStore.class);
bind(PersistentSafetyStateStore.class).to(BerkeleySafetyStateStore.class);
Multibinder.newSetBinder(binder(), NodeAutoCloseable.class)
.addBinding()
.to(BerkeleySafetyStateStore.class);
}

@Provides
@Singleton
BerkeleySafetyStateStore safetyStateStore(
RuntimeProperties properties,
Serialization serialization,
Metrics metrics,
@NodeStorageLocation String nodeStorageLocation) {
return new BerkeleySafetyStateStore(
serialization,
metrics,
nodeStorageLocation,
BerkeleyDbDefaults.createDefaultEnvConfigFromProperties(properties));
var binder = Multibinder.newSetBinder(binder(), NodeAutoCloseable.class);
binder.addBinding().to(PersistentSafetyStateStore.class);
binder.addBinding().to(AddressBookPersistence.class);
}

@Provides
@Singleton
BerkeleyAddressBookStore bdbAddressBookStore(
RuntimeProperties properties,
Serialization serialization,
Metrics metrics,
@NodeStorageLocation String nodeStorageLocation) {
return new BerkeleyAddressBookStore(
serialization,
metrics,
nodeStorageLocation,
BerkeleyDbDefaults.createDefaultEnvConfigFromProperties(properties));
EnvironmentConfig environmentConfig(RuntimeProperties properties) {
return BerkeleyDbDefaults.createDefaultEnvConfigFromProperties(properties);
}
},
Modules.override(new P2PModule(properties))
Expand Down

0 comments on commit a2829da

Please sign in to comment.