Build 49: auto-expire stale transactions; de-dupe receipt counterparty
Pending transactions that never complete now auto-cancel/expire after a configurable window (NostrConfig::expiry_secs, default 24h; lower it in nostr.toml to test). A sync-loop sweep (NostrService::expire_stale): - cancels stale outgoing sends + invoices we paid via GRIM's cancel_tx (WalletTask::Cancel), releasing the outputs they had locked; - annotates incoming payments / invoices we issued as Cancelled only (a late on-chain confirmation still wins); - marks pending incoming requests Expired. The activity feed and receipt now render "canceled" (this also fixes a latent bug where a manually-cancelled tx still showed "pending"). Receipt de-duplication: the To/From name rows are shown only when the counterparty has a real identity (petname or verified NIP-05); a bare npub now appears once, in the "nostr" row, instead of twice. 38 lib tests pass (2 new for the expiry bucket logic).
This commit is contained in:
@@ -16,7 +16,7 @@
|
||||
|
||||
use grin_wallet_libwallet::TxLogEntryType;
|
||||
|
||||
use crate::nostr::{Contact, NostrStore, TxNostrMeta};
|
||||
use crate::nostr::{Contact, NostrSendStatus, NostrStore, TxNostrMeta};
|
||||
use crate::wallet::Wallet;
|
||||
use crate::wallet::types::WalletTx;
|
||||
|
||||
@@ -28,6 +28,8 @@ pub struct ActivityItem {
|
||||
pub amount: u64,
|
||||
pub incoming: bool,
|
||||
pub confirmed: bool,
|
||||
/// Canceled/expired before completing (wallet-cancelled tx or expired meta).
|
||||
pub canceled: bool,
|
||||
pub system: bool,
|
||||
pub hue: usize,
|
||||
pub time: i64,
|
||||
@@ -47,6 +49,11 @@ pub struct ReceiptDetail {
|
||||
pub amount: u64,
|
||||
pub incoming: bool,
|
||||
pub confirmed: bool,
|
||||
/// Canceled/expired before completing.
|
||||
pub canceled: bool,
|
||||
/// Whether the counterparty has a real identity (petname / verified NIP-05)
|
||||
/// rather than just a bare npub. Gates the redundant To/From name rows.
|
||||
pub has_identity: bool,
|
||||
/// (current confirmations, required) when still pending and computable.
|
||||
pub confs: Option<(u64, u64)>,
|
||||
pub time: i64,
|
||||
@@ -109,6 +116,11 @@ pub fn receipt_detail(wallet: &Wallet, tx_id: u32) -> Option<ReceiptDetail> {
|
||||
_ => Some((0, data.info.minimum_confirmations)),
|
||||
}
|
||||
};
|
||||
let canceled = is_canceled(tx, meta.as_ref());
|
||||
let has_identity = meta
|
||||
.as_ref()
|
||||
.and_then(|m| store_ref.map(|s| has_real_identity(s, &m.npub)))
|
||||
.unwrap_or(false);
|
||||
Some(ReceiptDetail {
|
||||
tx_id,
|
||||
title,
|
||||
@@ -117,6 +129,8 @@ pub fn receipt_detail(wallet: &Wallet, tx_id: u32) -> Option<ReceiptDetail> {
|
||||
amount: tx.amount,
|
||||
incoming,
|
||||
confirmed: tx.data.confirmed,
|
||||
canceled,
|
||||
has_identity,
|
||||
confs,
|
||||
time,
|
||||
note,
|
||||
@@ -133,6 +147,33 @@ pub fn history_with(wallet: &Wallet, npub: &str) -> Vec<ActivityItem> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// True when a counterparty has a real, human identity (a local petname or a
|
||||
/// verified NIP-05) rather than just a bare npub. Used to suppress the
|
||||
/// redundant To/From name rows on the receipt when the name would just be the
|
||||
/// same truncated npub shown in the "nostr" row.
|
||||
pub fn has_real_identity(store: &NostrStore, npub: &str) -> bool {
|
||||
store
|
||||
.contact(npub)
|
||||
.map(|c| {
|
||||
c.petname.as_deref().map(|p| !p.is_empty()).unwrap_or(false)
|
||||
|| c.nip05_verified_at.is_some()
|
||||
})
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Whether a transaction was canceled/expired before completing: a wallet-level
|
||||
/// cancel (GRIM `TxSentCancelled`/`TxReceivedCancelled`), or expired nostr
|
||||
/// metadata while still unconfirmed (a late on-chain confirmation still wins).
|
||||
fn is_canceled(tx: &WalletTx, meta: Option<&TxNostrMeta>) -> bool {
|
||||
matches!(
|
||||
tx.data.tx_type,
|
||||
TxLogEntryType::TxSentCancelled | TxLogEntryType::TxReceivedCancelled
|
||||
) || (!tx.data.confirmed
|
||||
&& meta
|
||||
.map(|m| m.status == NostrSendStatus::Cancelled)
|
||||
.unwrap_or(false))
|
||||
}
|
||||
|
||||
/// Resolve the display title for a contact npub.
|
||||
pub fn contact_title(store: &NostrStore, npub: &str) -> (String, usize) {
|
||||
if let Some(contact) = store.contact(npub) {
|
||||
@@ -248,6 +289,7 @@ fn build_item(tx: &WalletTx, store: Option<&NostrStore>) -> ActivityItem {
|
||||
.or(Some(tx.data.creation_ts))
|
||||
.map(|t| t.timestamp())
|
||||
.unwrap_or(0);
|
||||
let canceled = is_canceled(tx, meta.as_ref());
|
||||
|
||||
ActivityItem {
|
||||
tx_id: tx.data.id,
|
||||
@@ -256,6 +298,7 @@ fn build_item(tx: &WalletTx, store: Option<&NostrStore>) -> ActivityItem {
|
||||
amount: tx.amount,
|
||||
incoming,
|
||||
confirmed: tx.data.confirmed,
|
||||
canceled,
|
||||
system,
|
||||
hue,
|
||||
time,
|
||||
|
||||
+26
-12
@@ -1102,7 +1102,16 @@ impl GoblinWalletView {
|
||||
w::kicker(ui, "Transaction details");
|
||||
ui.add_space(10.0);
|
||||
w::card(ui, |ui| {
|
||||
let (status, sub) = if d.confirmed {
|
||||
let (status, sub) = if d.canceled {
|
||||
(
|
||||
"Canceled",
|
||||
if d.incoming {
|
||||
"Expired".to_string()
|
||||
} else {
|
||||
"Funds returned".to_string()
|
||||
},
|
||||
)
|
||||
} else if d.confirmed {
|
||||
(
|
||||
"Complete",
|
||||
if d.incoming {
|
||||
@@ -1121,13 +1130,15 @@ impl GoblinWalletView {
|
||||
)
|
||||
};
|
||||
w::info_row(ui, status, &sub);
|
||||
let (to, from) = if d.incoming {
|
||||
("You".to_string(), d.title.clone())
|
||||
} else {
|
||||
(d.title.clone(), "You".to_string())
|
||||
};
|
||||
w::info_row(ui, "To", &to);
|
||||
w::info_row(ui, "From", &from);
|
||||
if d.has_identity {
|
||||
let (to, from) = if d.incoming {
|
||||
("You".to_string(), d.title.clone())
|
||||
} else {
|
||||
(d.title.clone(), "You".to_string())
|
||||
};
|
||||
w::info_row(ui, "To", &to);
|
||||
w::info_row(ui, "From", &from);
|
||||
}
|
||||
if let Some(npub) = &d.npub {
|
||||
w::info_row(ui, "nostr", &data::short_npub(npub));
|
||||
}
|
||||
@@ -1242,13 +1253,15 @@ impl GoblinWalletView {
|
||||
let sign = if item.incoming { "+ " } else { "− " };
|
||||
let amount =
|
||||
format!("{}{}{}", sign, w::amount_str(item.amount), w::TSU);
|
||||
let status_word =
|
||||
if item.canceled { "canceled" } else { "pending" };
|
||||
let subtitle = match (&item.note, item.confirmed) {
|
||||
(Some(n), true) => {
|
||||
format!("{} · {}", n, View::format_time(item.time))
|
||||
}
|
||||
(Some(n), false) => format!("{} · pending", n),
|
||||
(Some(n), false) => format!("{} · {}", n, status_word),
|
||||
(None, true) => View::format_time(item.time),
|
||||
(None, false) => "pending".to_string(),
|
||||
(None, false) => status_word.to_string(),
|
||||
};
|
||||
if w::activity_row(
|
||||
ui,
|
||||
@@ -1406,11 +1419,12 @@ impl GoblinWalletView {
|
||||
) {
|
||||
let sign = if item.incoming { "+ " } else { "− " };
|
||||
let amount = format!("{}{}{}", sign, w::amount_str(item.amount), w::TSU);
|
||||
let status_word = if item.canceled { "canceled" } else { "pending" };
|
||||
let subtitle = match (&item.note, item.confirmed) {
|
||||
(Some(note), true) => format!("{} · {}", note, View::format_time(item.time)),
|
||||
(Some(note), false) => format!("{} · pending", note),
|
||||
(Some(note), false) => format!("{} · {}", note, status_word),
|
||||
(None, true) => View::format_time(item.time),
|
||||
(None, false) => "pending".to_string(),
|
||||
(None, false) => status_word.to_string(),
|
||||
};
|
||||
let tex = self.handle_tex(ui.ctx(), wallet, &item.title);
|
||||
if w::activity_row(
|
||||
|
||||
+141
-1
@@ -12,7 +12,7 @@
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
//! Per-wallet nostr service: relay connections over the embedded Tor client,
|
||||
//! Per-wallet nostr service: relay connections over the Nym mixnet,
|
||||
//! identity event publishing, the guarded ingest loop and the DM send path.
|
||||
|
||||
use log::{error, info, warn};
|
||||
@@ -35,6 +35,7 @@ use crate::nostr::types::*;
|
||||
use crate::nostr::{NostrConfig, NostrIdentity, NostrStore};
|
||||
use crate::nym::NymWebSocketTransport;
|
||||
use crate::wallet::Wallet;
|
||||
use crate::wallet::types::WalletTask;
|
||||
|
||||
/// A peer's published nostr profile (kind-0 metadata), used to confirm a
|
||||
/// pasted key belongs to a live identity before paying it.
|
||||
@@ -287,6 +288,79 @@ impl NostrService {
|
||||
self.config.read().relays()
|
||||
}
|
||||
|
||||
/// Auto-expire stale pending transactions after the configured window
|
||||
/// (`NostrConfig::expiry_secs`, default 24h). A transaction that never
|
||||
/// completed is canceled/expired:
|
||||
/// - Outgoing sends and invoices we paid LOCK our outputs, so they are
|
||||
/// cancelled at the wallet level (reusing GRIM's `cancel_tx` via
|
||||
/// `WalletTask::Cancel`) to release those funds.
|
||||
/// - Incoming payments and invoices we issued lock nothing of ours, so we
|
||||
/// only annotate the metadata `Cancelled`; if a payment posts late,
|
||||
/// on-chain confirmation still wins (the UI only shows "canceled" while
|
||||
/// unconfirmed).
|
||||
/// - Pending incoming requests become `Expired`.
|
||||
///
|
||||
/// Runs from the wallet sync loop, so a lowered `expiry_secs` (set in
|
||||
/// `nostr.toml` for testing) takes effect within a sync cycle.
|
||||
pub fn expire_stale(&self, wallet: &Wallet) {
|
||||
let now = unix_time();
|
||||
let window = self.config.read().expiry_secs();
|
||||
if window <= 0 {
|
||||
return;
|
||||
}
|
||||
|
||||
let stale: Vec<TxNostrMeta> = self
|
||||
.store
|
||||
.all_tx_meta()
|
||||
.into_iter()
|
||||
.filter(|m| !expiry_terminal(m.status))
|
||||
.filter(|m| now - m.created_at > window)
|
||||
.collect();
|
||||
|
||||
if !stale.is_empty() {
|
||||
// Map slate uuid → wallet tx id once (public wallet data), so we can
|
||||
// cancel the underlying GRIM tx for the funds-locking cases.
|
||||
let tx_ids: HashMap<String, u32> = wallet
|
||||
.get_data()
|
||||
.and_then(|d| d.txs)
|
||||
.map(|txs| {
|
||||
txs.iter()
|
||||
.filter_map(|t| t.data.tx_slate_id.map(|u| (u.to_string(), t.data.id)))
|
||||
.collect()
|
||||
})
|
||||
.unwrap_or_default();
|
||||
|
||||
for meta in stale {
|
||||
// Only outgoing sends + invoices we paid lock our outputs.
|
||||
if expiry_locks_outputs(meta.direction, meta.status) {
|
||||
if let Some(&tx_id) = tx_ids.get(&meta.slate_id) {
|
||||
info!(
|
||||
"nostr: expiring stale send {} → cancel wallet tx {}",
|
||||
meta.slate_id, tx_id
|
||||
);
|
||||
wallet.task(WalletTask::Cancel(tx_id));
|
||||
}
|
||||
} else {
|
||||
info!(
|
||||
"nostr: expiring stale {} ({:?})",
|
||||
meta.slate_id, meta.direction
|
||||
);
|
||||
}
|
||||
self.store
|
||||
.update_tx_status(&meta.slate_id, NostrSendStatus::Cancelled);
|
||||
}
|
||||
}
|
||||
|
||||
// Incoming payment requests we never approved.
|
||||
for req in self.store.pending_requests() {
|
||||
if now - req.received_at > window {
|
||||
info!("nostr: expiring stale incoming request {}", req.rumor_id);
|
||||
self.store
|
||||
.update_request_status(&req.rumor_id, RequestStatus::Expired);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Sliding-window rate limiter, true when the event is allowed.
|
||||
fn allow_sender(&self, sender: &str, is_contact: bool) -> bool {
|
||||
let max = if is_contact {
|
||||
@@ -557,6 +631,31 @@ async fn publish_identity(svc: &Arc<NostrService>, client: &Client) {
|
||||
}
|
||||
}
|
||||
|
||||
/// A transaction in a terminal state never expires (already done or canceled).
|
||||
fn expiry_terminal(status: NostrSendStatus) -> bool {
|
||||
matches!(
|
||||
status,
|
||||
NostrSendStatus::Finalized | NostrSendStatus::Cancelled
|
||||
)
|
||||
}
|
||||
|
||||
/// Whether an expired transaction with this (direction, status) locked OUR
|
||||
/// outputs and therefore needs a wallet-level `cancel_tx` to release them
|
||||
/// (outgoing sends and invoices we paid). Incoming payments and invoices we
|
||||
/// issued lock nothing of ours, so those are only annotated as canceled.
|
||||
fn expiry_locks_outputs(direction: NostrTxDirection, status: NostrSendStatus) -> bool {
|
||||
matches!(
|
||||
(direction, status),
|
||||
(NostrTxDirection::Sent, NostrSendStatus::Created)
|
||||
| (NostrTxDirection::Sent, NostrSendStatus::AwaitingS2)
|
||||
| (NostrTxDirection::Sent, NostrSendStatus::SendFailed)
|
||||
| (
|
||||
NostrTxDirection::RequestedOfUs,
|
||||
NostrSendStatus::PaidAwaitingFinalize
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
/// Re-dispatch our pending outgoing messages (crash/offline recovery).
|
||||
async fn reconcile(svc: &Arc<NostrService>, wallet: &Wallet) {
|
||||
let now = unix_time();
|
||||
@@ -834,3 +933,44 @@ async fn handle_wrap(svc: &Arc<NostrService>, wallet: &Wallet, client: &Client,
|
||||
svc.store.mark_processed(&rumor_id);
|
||||
svc.store.mark_processed(&slate_marker);
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn terminal_states_do_not_expire() {
|
||||
assert!(expiry_terminal(NostrSendStatus::Finalized));
|
||||
assert!(expiry_terminal(NostrSendStatus::Cancelled));
|
||||
// Everything in flight is eligible to expire.
|
||||
for s in [
|
||||
NostrSendStatus::Created,
|
||||
NostrSendStatus::AwaitingS2,
|
||||
NostrSendStatus::ReceivedNoReply,
|
||||
NostrSendStatus::RepliedS2,
|
||||
NostrSendStatus::AwaitingI2,
|
||||
NostrSendStatus::PaidAwaitingFinalize,
|
||||
NostrSendStatus::SendFailed,
|
||||
] {
|
||||
assert!(!expiry_terminal(s), "{s:?} should be expirable");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn only_our_committed_outputs_get_cancelled() {
|
||||
use NostrSendStatus::*;
|
||||
use NostrTxDirection::*;
|
||||
// Our sends (we locked outputs) and invoices we paid → cancel to unlock.
|
||||
assert!(expiry_locks_outputs(Sent, Created));
|
||||
assert!(expiry_locks_outputs(Sent, AwaitingS2));
|
||||
assert!(expiry_locks_outputs(Sent, SendFailed));
|
||||
assert!(expiry_locks_outputs(RequestedOfUs, PaidAwaitingFinalize));
|
||||
// Incoming payments and invoices we issued lock nothing of ours →
|
||||
// annotate only, never cancel a tx that could still settle/pay.
|
||||
assert!(!expiry_locks_outputs(Received, ReceivedNoReply));
|
||||
assert!(!expiry_locks_outputs(Received, RepliedS2));
|
||||
assert!(!expiry_locks_outputs(RequestedByUs, AwaitingI2));
|
||||
assert!(!expiry_locks_outputs(RequestedByUs, Created));
|
||||
assert!(!expiry_locks_outputs(RequestedOfUs, ReceivedNoReply));
|
||||
}
|
||||
}
|
||||
|
||||
+7
-5
@@ -42,8 +42,9 @@ pub struct NostrConfig {
|
||||
accept_from: Option<AcceptPolicy>,
|
||||
/// NIP-05 identity server base URL.
|
||||
nip05_server: Option<String>,
|
||||
/// Days after which a pending outgoing payment is shown as expired.
|
||||
request_expiry_days: Option<u64>,
|
||||
/// Seconds after which a still-pending transaction is auto-canceled/expired.
|
||||
/// Default 24h; lower it (e.g. 60) in nostr.toml to test the expiry flow.
|
||||
expiry_secs: Option<i64>,
|
||||
/// Whether incoming payment requests (Invoice1) are accepted. Opt-out: on
|
||||
/// by default. When off, incoming requests are dropped and the preference is
|
||||
/// advertised in our kind-0 profile so requesters see it before sending.
|
||||
@@ -61,7 +62,7 @@ impl Default for NostrConfig {
|
||||
relays: None,
|
||||
accept_from: None,
|
||||
nip05_server: None,
|
||||
request_expiry_days: None,
|
||||
expiry_secs: None,
|
||||
allow_incoming_requests: None,
|
||||
path: None,
|
||||
}
|
||||
@@ -124,8 +125,9 @@ impl NostrConfig {
|
||||
.unwrap_or_else(|| DEFAULT_NIP05_SERVER.to_string())
|
||||
}
|
||||
|
||||
pub fn request_expiry_days(&self) -> u64 {
|
||||
self.request_expiry_days.unwrap_or(7)
|
||||
/// Seconds after which a still-pending transaction is auto-canceled/expired.
|
||||
pub fn expiry_secs(&self) -> i64 {
|
||||
self.expiry_secs.unwrap_or(24 * 60 * 60)
|
||||
}
|
||||
|
||||
pub fn allow_incoming_requests(&self) -> bool {
|
||||
|
||||
@@ -192,6 +192,14 @@ impl NostrStore {
|
||||
self.all_json(&self.requests)
|
||||
}
|
||||
|
||||
/// Update the status of an existing payment request.
|
||||
pub fn update_request_status(&self, rumor_id: &str, status: RequestStatus) {
|
||||
if let Some(mut req) = self.request(rumor_id) {
|
||||
req.status = status;
|
||||
self.save_request(&req);
|
||||
}
|
||||
}
|
||||
|
||||
pub fn pending_requests(&self) -> Vec<PaymentRequest> {
|
||||
let mut reqs: Vec<PaymentRequest> = self
|
||||
.all_requests()
|
||||
|
||||
@@ -1904,6 +1904,9 @@ fn start_sync(wallet: Wallet) -> Thread {
|
||||
// Start nostr payment-messaging service (idempotent).
|
||||
if let Some(service) = wallet.nostr_service() {
|
||||
service.start(wallet.clone());
|
||||
// Auto-cancel/expire transactions left pending past the
|
||||
// configured window (frees outputs locked by stale sends).
|
||||
service.expire_stale(&wallet);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user