Db service by zealsham · Pull Request #1361 · payjoin/rust-payjoin

zealsham · 2026-02-26T01:03:18Z

This PR addresses part of #941 by making Db a tower service. Since payjoin-directory will eventually be deprecated, this PR moves the db implementation to payjoin-mailroom while still allowing the directory binary to be built if needed. The PR also ensures directory::Service remains compatible via the Db trait adapter.

Some parts of these PR was bootstrapped using chatgpt web

Pull Request Checklist

Please confirm the following before requesting review:

I have disclosed my use of
AI
in the body of this PR.
I have read CONTRIBUTING.md and rebased my branch to produce hygienic commits.

benalleng · 2026-02-26T16:13:03Z

payjoin-directory/src/lib.rs

+        async fn wait_for_v2_payload(
+            &self,
+            id: &ShortId,
+        ) -> Result<Arc<Vec<u8>>, DbError<Self::OperationalError>> {
+            let receiver = {
+                let mut state = self.state.lock().await;
+                if let Some(payload) = state.v2_payloads.get(id) {
+                    return Ok(payload.clone());
+                }
+                if let Some((ref mut payload, _response_tx)) = state.v1_pending.get_mut(id) {
+                    if let Some(p) = payload.take() {
+                        return Ok(p);
+                    }
+                }
+                let (tx, rx) = oneshot::channel();
+                state.v2_waiters.insert(*id, tx);
+                rx
+            };
+            match tokio::time::timeout(self.timeout, receiver).await {
+                Ok(Ok(payload)) => Ok(payload),
+                Ok(Err(_)) => Err(DbError::Operational(std::io::Error::new(
+                    std::io::ErrorKind::ConnectionReset,
+                    "closed",
+                ))),
+                Err(elapsed) => Err(DbError::Timeout(elapsed)),
+            }
+        }


In this function we just return an error on a Sender error or timeout but the TestDbState keeps a v2_waiters with this ShortId. Is there some value in mimicing the behavior in the FilesDb where the v2_waiter is cleared no matter what?

I ask this only if others see value in a long running mailroom test, say to demonstrate an overfilling mailroom.

benalleng · 2026-02-26T16:13:46Z

payjoin-directory/src/lib.rs

+            match tokio::time::timeout(self.timeout, receiver).await {
+                Ok(Ok(payload)) => Ok(Arc::new(payload)),
+                Ok(Err(_)) => Err(DbError::V1SenderUnavailable),
+                Err(elapsed) => Err(DbError::Timeout(elapsed)),
+            }
+        }
+    }


Same here where the v1_pending is not cleared on an error

spacebear21

This is looking promising! I left some high-level feedback while this is still in draft. Another question I had: do you expect this do be backwards-compatible with an existing DB? Say I started running a payjoin-mailroom yesterday and have active sessions, and upgrade to this, will it keep existing sessions around?

spacebear21 · 2026-02-26T23:50:20Z

payjoin-mailroom/src/bin_directory.rs

We can kill the standalone directory binary entirely

payjoin-mailroom/src/db/mod.rs

spacebear21 · 2026-02-26T23:54:37Z

payjoin-mailroom/src/db/files.rs

+    pub(crate) capacity: usize,
    persistent_storage: DiskStorage,
-    pending_v1: HashMap<ShortId, V1WaitMapEntry>,
+    pub(crate) pending_v1: HashMap<ShortId, V1WaitMapEntry>,
    pending_v2: HashMap<ShortId, V2WaitMapEntry>,
-    insert_order: VecDeque<(SystemTime, ShortId)>,
-    read_order: VecDeque<(SystemTime, ShortId)>,
+    pub(crate) insert_order: VecDeque<(SystemTime, ShortId)>,
+    pub(crate) read_order: VecDeque<(SystemTime, ShortId)>,
    read_mailbox_ids: HashSet<ShortId>,
-    unread_ttl_below_capacity: Duration,
-    unread_ttl_at_capacity: Duration,
-    read_ttl: Duration,
+    pub(crate) unread_ttl_below_capacity: Duration,
+    pub(crate) unread_ttl_at_capacity: Duration,
+    pub(crate) read_ttl: Duration,


Why did these change to pub(crate)? I don't see where they are being used.

oops!, when i started writting this , i was importing types i needed from the payjoin-directory crate, made them pub crate there and copied them over here.

will change it

looks like this is still unresolved in the latest push? there are a few other pub(crate) additions throughout this file which also seem unnecessary.

spacebear21 · 2026-02-26T23:57:10Z

payjoin-directory/src/db/mod.rs

is there a reason to keep the payjoin-directory db mod?

spacebear21 · 2026-02-26T23:57:52Z

payjoin-directory/src/lib.rs

-        let db = FilesDb::init(Duration::from_millis(100), dir.keep()).await.expect("db init");
+    /// In-memory Db implementation for directory service tests.
+    #[derive(Clone)]
+    struct TestDb {


Similarly, would it be more appropriate to have these tests live in payjoin-mailroom's db mod?

coveralls · 2026-03-04T10:15:08Z

Pull Request Test Coverage Report for Build 22786494612

Details

149 of 187 (79.68%) changed or added relevant lines in 6 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+1.0%) to 83.507%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
payjoin-mailroom/src/access_control.rs	0	1	0.0%
payjoin-mailroom/src/db/files.rs	18	20	90.0%
payjoin-mailroom/src/lib.rs	11	14	78.57%
payjoin-mailroom/src/directory.rs	39	47	82.98%
payjoin-mailroom/src/db/mod.rs	80	104	76.92%

Totals
Change from base Build 22741440695:	1.0%
Covered Lines:	10688
Relevant Lines:	12799

💛 - Coveralls

spacebear21

Epic net deletion!

cACK on deleting the payjoin-directory crate and moving it to a service with axum routing.

It looks like the LLM was a bit overzealous in deleting docstrings and comments, tests, and random line breaks. The latter is fine, but the tests and comments need to be restored where applicable.

Lastly, can you please clean up the commit message per the contributing guidelines?

Thanks for taking on this big refactor.

spacebear21 · 2026-03-04T22:30:33Z

payjoin-mailroom/src/db/files.rs

+    pub(crate) capacity: usize,
    persistent_storage: DiskStorage,
-    pending_v1: HashMap<ShortId, V1WaitMapEntry>,
+    pub(crate) pending_v1: HashMap<ShortId, V1WaitMapEntry>,
    pending_v2: HashMap<ShortId, V2WaitMapEntry>,
-    insert_order: VecDeque<(SystemTime, ShortId)>,
-    read_order: VecDeque<(SystemTime, ShortId)>,
+    pub(crate) insert_order: VecDeque<(SystemTime, ShortId)>,
+    pub(crate) read_order: VecDeque<(SystemTime, ShortId)>,
    read_mailbox_ids: HashSet<ShortId>,
-    unread_ttl_below_capacity: Duration,
-    unread_ttl_at_capacity: Duration,
-    read_ttl: Duration,
+    pub(crate) unread_ttl_below_capacity: Duration,
+    pub(crate) unread_ttl_at_capacity: Duration,
+    pub(crate) read_ttl: Duration,


looks like this is still unresolved in the latest push? there are a few other pub(crate) additions throughout this file which also seem unnecessary.

payjoin-directory/src/lib.rs

payjoin-mailroom/src/directory.rs

payjoin-mailroom/src/key_config.rs

payjoin-directory/contrib/health-check.sh

spacebear21 · 2026-03-04T23:06:34Z

Also I just realized we reference payjoin-directory in various links through the codebase, for example in the README. We should grep the whole codebase for payjoin-directory to ensure no dangling links.

This pr addresses a part of payjoin#941 consigned with making Db a tower service. It also removes the payjoin-directory crate and moves all directory functionality into the payjoin-mailroom crate as a tower service.

spacebear21

ACK f2d07db

I took the liberty to force push to fix a few minor things:

added back more comments that had been inadvertently removed
changed the payjoin dependency in payjoin-mailroom to rc.2 instead of rc.1
cleaned up the commit message a bit for typos/grammar

We'll need an ACK from someone else to merge this because I was the last to push on the PR so my approval doesn't count per github rules.

zealsham marked this pull request as draft February 26, 2026 01:03

zealsham force-pushed the db-service branch 4 times, most recently from f80134e to a91e3e0 Compare February 26, 2026 14:06

benalleng reviewed Feb 26, 2026

View reviewed changes

spacebear21 reviewed Feb 27, 2026

View reviewed changes

zealsham mentioned this pull request Mar 2, 2026

Standup Input: Week of 2026-03-02 #1372

Closed

zealsham force-pushed the db-service branch from ebe7bc1 to 021d21d Compare March 4, 2026 10:10

zealsham force-pushed the db-service branch 2 times, most recently from fde034f to f9b75aa Compare March 4, 2026 10:36

zealsham requested review from nothingmuch and spacebear21 March 4, 2026 10:56

zealsham marked this pull request as ready for review March 4, 2026 10:57

spacebear21 requested changes Mar 4, 2026

View reviewed changes

spacebear21 reviewed Mar 4, 2026

View reviewed changes

payjoin-directory/contrib/health-check.sh Outdated Show resolved Hide resolved

zealsham force-pushed the db-service branch 2 times, most recently from 35ad2c2 to 44fda1e Compare March 6, 2026 00:56

spacebear21 force-pushed the db-service branch from 44fda1e to a8c5d95 Compare March 6, 2026 23:26

Implement Directory and its db as a tower-service

f2d07db

This pr addresses a part of payjoin#941 consigned with making Db a tower service. It also removes the payjoin-directory crate and moves all directory functionality into the payjoin-mailroom crate as a tower service.

spacebear21 force-pushed the db-service branch from a8c5d95 to f2d07db Compare March 6, 2026 23:37

spacebear21 approved these changes Mar 6, 2026

View reviewed changes

Conversation

zealsham commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 22786494612

Details

💛 - Coveralls

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

spacebear21 commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spacebear21 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zealsham commented Feb 26, 2026 •

edited

Loading

coveralls commented Mar 4, 2026 •

edited

Loading

spacebear21 commented Mar 4, 2026 •

edited

Loading

spacebear21 left a comment •

edited

Loading