Skip to content

Commit

Permalink
Fast construction of Transaction when reading from the chain db
Browse files Browse the repository at this point in the history
Transactions that contains mints involve parsing assets, which in turn
involve parsing public addresses of the owner for those assets. Parsing
a public address is currently an expensive operation, because it
involves elliptic curve point multiplications.

Transactions containing mints are therefore harmful for the performance
of wallet scans.

To improve the performance of wallet scans, this commit:
- introduces a new `PublicAddress::new_unchecked` (in Rust) constructor
  that speeds up construction of public addresses from trusted input;
- adds a `skipValidation` parameter to `Transaction` (in TypeScript) to
  allow using the "unchecked" variants of constructors;
- changes the chain db deserializer to read transactions using
  `skipValidation: true`.

Note that, while the goal of this commit is to improve wallet scan
performance, the impact of this commit is actually broader: it impacts
all reads from the chain db. This is good for performance, but may be a
concern for security and stability. The assumption is that if something
is stored in the chain db, then it was previously validated and
therefore it can be trusted.
  • Loading branch information
andiflabs committed Jun 26, 2024
1 parent 6107b89 commit 6a9d58f
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 18 deletions.
2 changes: 1 addition & 1 deletion ironfish-rust-nodejs/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export class Asset {
static nativeId(): Buffer
id(): Buffer
serialize(): Buffer
static deserialize(jsBytes: Buffer): NativeAsset
static deserialize(jsBytes: Buffer, skipValidation?: boolean | undefined | null): NativeAsset
}
export type NativeNoteEncrypted = NoteEncrypted
export class NoteEncrypted {
Expand Down
10 changes: 8 additions & 2 deletions ironfish-rust-nodejs/src/structs/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,15 @@ impl NativeAsset {
}

#[napi(factory)]
pub fn deserialize(js_bytes: JsBuffer) -> Result<Self> {
pub fn deserialize(js_bytes: JsBuffer, skip_validation: Option<bool>) -> Result<Self> {
let bytes = js_bytes.into_value()?;
let asset = Asset::read(bytes.as_ref()).map_err(to_napi_err)?;
let skip_validation = skip_validation.unwrap_or(false);
let asset = if !skip_validation {
Asset::read(bytes.as_ref())
} else {
Asset::read_unchecked(bytes.as_ref())
}
.map_err(to_napi_err)?;

Ok(NativeAsset { asset })
}
Expand Down
14 changes: 14 additions & 0 deletions ironfish-rust/src/assets/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,20 @@ impl Asset {
Asset::new_with_nonce(creator, name, metadata, nonce)
}

pub fn read_unchecked<R: io::Read>(mut reader: R) -> Result<Self, IronfishError> {
let creator = PublicAddress::read_unchecked(&mut reader)?;

let mut name = [0; NAME_LENGTH];
reader.read_exact(&mut name[..])?;

let mut metadata = [0; METADATA_LENGTH];
reader.read_exact(&mut metadata[..])?;

let nonce = reader.read_u8()?;

Asset::new_with_nonce(creator, name, metadata, nonce)
}

/// Stow the bytes of this struct in the given writer.
pub fn write<W: io::Write>(&self, mut writer: W) -> Result<(), IronfishError> {
self.creator.write(&mut writer)?;
Expand Down
29 changes: 18 additions & 11 deletions ironfish-rust/src/keys/public_address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,18 @@ pub struct PublicAddress(pub(crate) SubgroupPoint);

impl PublicAddress {
/// Initialize a public address from its 32 byte representation.
pub fn new(
public_address_bytes: &[u8; PUBLIC_ADDRESS_SIZE],
) -> Result<PublicAddress, IronfishError> {
assert!(public_address_bytes.len() == 32);
let public_address_non_prime = SubgroupPoint::from_bytes(public_address_bytes);

if public_address_non_prime.is_some().into() {
Ok(PublicAddress(public_address_non_prime.unwrap()))
} else {
Err(IronfishError::new(IronfishErrorKind::InvalidPaymentAddress))
}
pub fn new(bytes: &[u8; PUBLIC_ADDRESS_SIZE]) -> Result<Self, IronfishError> {
Option::from(SubgroupPoint::from_bytes(bytes))
.map(PublicAddress)
.ok_or_else(|| IronfishError::new(IronfishErrorKind::InvalidPaymentAddress))
}

/// Initialize a public address from its 32 byte representation, without performing expensive
/// checks on the validity of the address.
pub fn new_unchecked(bytes: &[u8; PUBLIC_ADDRESS_SIZE]) -> Result<Self, IronfishError> {
Option::from(SubgroupPoint::from_bytes_unchecked(bytes))
.map(PublicAddress)
.ok_or_else(|| IronfishError::new(IronfishErrorKind::InvalidPaymentAddress))
}

/// Load a public address from a Read implementation (e.g: socket, file)
Expand All @@ -43,6 +44,12 @@ impl PublicAddress {
Self::new(&address_bytes)
}

pub fn read_unchecked<R: io::Read>(reader: &mut R) -> Result<Self, IronfishError> {
let mut address_bytes = [0; PUBLIC_ADDRESS_SIZE];
reader.read_exact(&mut address_bytes)?;
Self::new_unchecked(&address_bytes)
}

/// Initialize a public address from a sapling key. Typically constructed from
/// SaplingKey::public_address()
pub fn from_key(sapling_key: &SaplingKey) -> PublicAddress {
Expand Down
2 changes: 1 addition & 1 deletion ironfish/src/blockchain/database/transactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export class TransactionsValueEncoding implements IDatabaseEncoding<Transactions
const transactions = []

while (reader.left()) {
transactions.push(new Transaction(reader.readVarBytes()))
transactions.push(new Transaction(reader.readVarBytes(), { skipValidation: true }))
}

return { transactions }
Expand Down
6 changes: 3 additions & 3 deletions ironfish/src/primitives/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export class Transaction {
private transactionPosted: TransactionPosted | null = null
private referenceCount = 0

constructor(transactionPostedSerialized: Buffer) {
constructor(transactionPostedSerialized: Buffer, options?: { skipValidation?: boolean }) {
this.transactionPostedSerialized = transactionPostedSerialized

const reader = bufio.read(this.transactionPostedSerialized, true)
Expand Down Expand Up @@ -109,14 +109,14 @@ export class Transaction {
reader.seek(PROOF_LENGTH)

// output note
return new NoteEncrypted(reader.readBytes(ENCRYPTED_NOTE_LENGTH, true))
return new NoteEncrypted(reader.readBytes(ENCRYPTED_NOTE_LENGTH, true), options)
})

this.mints = Array.from({ length: _mintsLength }, () => {
// proof
reader.seek(PROOF_LENGTH)

const asset = Asset.deserialize(reader.readBytes(ASSET_LENGTH))
const asset = Asset.deserialize(reader.readBytes(ASSET_LENGTH), options?.skipValidation)
const value = reader.readBigU64()

let owner = null
Expand Down

0 comments on commit 6a9d58f

Please sign in to comment.