Update memo retrieval API to reflect that memos may not be present.

Memos may be absent for both sent and received notes in cases where only
compact block information has been used to populate the wallet database.
This fixes a potential crash in the case that we attempt to decode a
SQLite `NULL` as a byte array. It does, however, introduce a slight
semantic confusion that will need to be considered in the case of future
updates where a note may not have an associated memo; at present, the
only reason we might not have the memo is that we might not have
retrieved the full transaction information from the chain, but in the
future there might be other possible reasons for this absence.

Fixes #384
This commit is contained in:
Kris Nuttycombe 2023-05-16 10:27:40 -06:00
parent bc55893267
commit d99b4d4d6e
3 changed files with 29 additions and 16 deletions

View File

@ -155,8 +155,10 @@ pub trait WalletRead {
/// Returns the memo for a note.
///
/// Implementations of this method must return an error if the note identifier
/// does not appear in the backing data store.
fn get_memo(&self, id_note: Self::NoteRef) -> Result<Memo, Self::Error>;
/// does not appear in the backing data store. Returns `Ok(None)` if the note
/// is known to the wallet but memo data has not yet been populated for that
/// note.
fn get_memo(&self, id_note: Self::NoteRef) -> Result<Option<Memo>, Self::Error>;
/// Returns a transaction.
fn get_transaction(&self, id_tx: Self::TxRef) -> Result<Transaction, Self::Error>;
@ -512,8 +514,8 @@ pub mod testing {
Ok(Amount::zero())
}
fn get_memo(&self, _id_note: Self::NoteRef) -> Result<Memo, Self::Error> {
Ok(Memo::Empty)
fn get_memo(&self, _id_note: Self::NoteRef) -> Result<Option<Memo>, Self::Error> {
Ok(None)
}
fn get_transaction(&self, _id_tx: Self::TxRef) -> Result<Transaction, Self::Error> {

View File

@ -190,7 +190,7 @@ impl<P: consensus::Parameters> WalletRead for WalletDb<P> {
wallet::get_transaction(self, id_tx)
}
fn get_memo(&self, id_note: Self::NoteRef) -> Result<Memo, Self::Error> {
fn get_memo(&self, id_note: Self::NoteRef) -> Result<Option<Memo>, Self::Error> {
match id_note {
NoteId::SentNoteId(id_note) => wallet::get_sent_memo(self, id_note),
NoteId::ReceivedNoteId(id_note) => wallet::get_received_memo(self, id_note),
@ -349,7 +349,7 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> {
self.wallet_db.get_transaction(id_tx)
}
fn get_memo(&self, id_note: Self::NoteRef) -> Result<Memo, Self::Error> {
fn get_memo(&self, id_note: Self::NoteRef) -> Result<Option<Memo>, Self::Error> {
self.wallet_db.get_memo(id_note)
}

View File

@ -480,17 +480,21 @@ pub(crate) fn get_balance_at<P>(
pub(crate) fn get_received_memo<P>(
wdb: &WalletDb<P>,
id_note: i64,
) -> Result<Memo, SqliteClientError> {
let memo_bytes: Vec<_> = wdb.conn.query_row(
) -> Result<Option<Memo>, SqliteClientError> {
let memo_bytes: Option<Vec<_>> = wdb.conn.query_row(
"SELECT memo FROM sapling_received_notes
WHERE id_note = ?",
[id_note],
|row| row.get(0),
)?;
MemoBytes::from_bytes(&memo_bytes)
.and_then(Memo::try_from)
.map_err(SqliteClientError::from)
memo_bytes
.map(|b| {
MemoBytes::from_bytes(&b)
.and_then(Memo::try_from)
.map_err(SqliteClientError::from)
})
.transpose()
}
/// Looks up a transaction by its internal database identifier.
@ -519,17 +523,24 @@ pub(crate) fn get_transaction<P: Parameters>(
///
/// The note is identified by its row index in the `sent_notes` table within the wdb
/// database.
pub(crate) fn get_sent_memo<P>(wdb: &WalletDb<P>, id_note: i64) -> Result<Memo, SqliteClientError> {
let memo_bytes: Vec<_> = wdb.conn.query_row(
pub(crate) fn get_sent_memo<P>(
wdb: &WalletDb<P>,
id_note: i64,
) -> Result<Option<Memo>, SqliteClientError> {
let memo_bytes: Option<Vec<_>> = wdb.conn.query_row(
"SELECT memo FROM sent_notes
WHERE id_note = ?",
[id_note],
|row| row.get(0),
)?;
MemoBytes::from_bytes(&memo_bytes)
.and_then(Memo::try_from)
.map_err(SqliteClientError::from)
memo_bytes
.map(|b| {
MemoBytes::from_bytes(&b)
.and_then(Memo::try_from)
.map_err(SqliteClientError::from)
})
.transpose()
}
/// Returns the minimum and maximum heights for blocks stored in the wallet database.