feat(kromer): implement subscriptions by qrmcat · Pull Request #9 · ReconnectedCC/KromerSharp

qrmcat · 2026-04-27T14:17:24Z

Adds kromer V1 subscription contracts.
Contracts are owned by names/metanames. Creating/deleting a contract requires the name owner's privatekey. When a name is transferred, contract ownership and future payments transfer with it.
Subscriptions immediately charge then recur using a background billing worker.

New api endpoints are under /api/v1/subscriptions:

POST /api/v1/subscriptions
- body: { "privatekey", "name", "price", "period", "description" }
- for the name it accepts either a name, or metaname. Eg.: foo, foo.kro, shop@foo.kro
GET /api/v1/subscriptions/{id}?address=...
GET /api/v1/subscriptions?address=...&name=...&exclude_owned=...&only_owned=...&only_unsubscribable=...&limit=...&offset=...
POST /api/v1/subscriptions/{id}/subscribe
- body: { "privatekey": "..." }
POST /api/v1/subscriptions/{id}/unsubscribe
- body: { "privatekey": "..." }
DELETE /api/v1/subscriptions/{id}
- body: { "privatekey": "..." }

In the database, it adds subscription_status enum, subscription_contracts, and wallet_subscriptions
Added a hand-built migration using entity framework core (pls test it rigorously, it seemed to work but can never be sure enough)

The API is based mostly on this: https://rentry.co/px7k93mf

Laincy

I don't know enough about C# to comment on the actual quality so I'm sticking to general architectural stuff. Overall looks pretty good, Could perhaps use some more enforcing of invariants in the DB schema but I also don't know enough about Entity Framework to speak on how viable that actually is.

Laincy · 2026-04-27T14:42:24Z

Looks like this only processes the first 100 subscriptions that have lapsed, and only does so every minute. This probably won't cause many issues at runtime, but may cause processing to lag behind what's expected in cases that more than 100 have lapsed since the last sync.

Because this service also handles processing any subscriptions that would have lapsed during downtime, one could imagine this causing multiple minutes worth of backup depending on the amount of time that's passed and how many subscriptions there are.

Should be fixable by looping either here or in BillDueSubscriptionsAsync until the number of processed transactions is less than 100.

You could consider parallelising or multithreading.

Laincy · 2026-04-27T15:03:40Z

+
+        try
+        {
+            await ChargeSubscriptionAsync(contract, subscription, subscriber, owner, now);


Having the new subscription start whenever its processed may cause issues with what service providers expect, especially for shorter subscriptions. This is similar to what I mentioned on the billing service and won't apply in most cases but it may also cause issues when re-syncing after downtime. It might be better to start them from whenever the previous period should have ended.

This also causes issues when re-syncing, because it doesn't account for any periods that should have occurred during downtime. It was discussed ages ago so you wouldn't know but we were going to treat these periods as if they had happened and billed subscribers for it. I don't see any issues with changing that to something else if you think it makes more sense but it's worth a discussion.

Laincy · 2026-04-27T15:22:28Z

+                .Select(q => q.Name);
+
+            var subscribedContracts = context.WalletSubscriptions
+                .Where(q => q.WalletAddress == address && q.Status == SubscriptionStatus.Active);


Subscribers that cancel their subscription while there is still time remaining should still be counted as subscribers and receive benefits for the remaining time on their subscription. Whenever we fetch active subscribers it should be done in a way that preserves this (e.g. subscriptions who's next_payment is before now)

Commenting this here but this applies across the entire PR

Laincy · 2026-04-27T15:44:06Z

Not a critique of the implementation itself, but it would be nice to have more options for customizing who can subscribe to a contract:

Maximum number of subscribers
Restrict subscribers to a list of wallets or one specific wallet
Allow contracts to be closed, but not deleted. Existing subscribers can continue their subscription but no new subscribers.

qrmcat · 2026-04-27T18:58:20Z

@Laincy I think I've fixed most of the problems you mentioned! Care to take a peek again?

qrmcat · 2026-04-27T19:01:48Z

As for what you've mentioned here above:

What would the use-case for restricting subscribers to a list of wallets / one specific wallet be?

As for the other points, I'll implement them.

dimaguy · 2026-04-27T19:21:45Z

As for what you've mentioned here above:
* What would the use-case for restricting subscribers to a list of wallets / one specific wallet be?
As for the other points, I'll implement them.

use-case for restricting subscribers is for limited-access stuff, you wouldn't want people subscribing to stuff they aren't meant to, some programs might not deal well with that

qrmcat · 2026-04-27T19:23:27Z

Right. I'll implement that too aswell then

… migration not being visible

qrmcat · 2026-04-29T08:07:19Z

Implemented the subscription access control stuff: max subscriber caps, wallet allowlists, and closing contracts to new subscribers

Twijn · 2026-05-07T19:32:51Z

Here are a few issues I've noticed while trying to integrate subscriptions into Krawlet:

Transactions created from subscriptions are sent only to the address specified in the subscription, not to the name (or meta + name combo). Is this intended? See https://kromer.herrkatze.com/api/krist/transactions/11128 and https://kromer.herrkatze.com/api/v1/subscriptions/1
Rather than returning the "owns" and "subscribed" booleans, could we instead return the owned address and the subscriber's address, and allow multiple addresses or names to be queried?
- With the current structure, I can't retrieve all subscriptions for all user wallets without making a request per wallet.
- The owns/subscribed booleans are awkward to handle, especially because Krawlet often displays data in components that aren't associated with where the request is made and doesn't always have easy access to the address or name specified in the request.

bananasov

Code looks good to me, missing a few doc comments on the new routes and some general comments on the code.

qrmcat · 2026-08-30T12:19:47Z

meow I think I fixed everything

bananasov

LGTM

Ale32bit

Not too sure about the required keyword changes. Test it of course.

Ale32bit · 2026-08-31T14:47:27Z

+
+public class WalletSubscriptionDto
+{
+    public string Address { get; set; } = null!;


Suggested change

public string Address { get; set; } = null!;

public required string Address { get; set; }

Ale32bit · 2026-08-31T14:47:58Z

+{
+    public int Id { get; set; }
+
+    public string Receiver { get; set; } = null!;


Suggested change

public string Receiver { get; set; } = null!;

public required string Receiver { get; set; }

Ale32bit · 2026-08-31T14:48:14Z

+
+    public string Receiver { get; set; } = null!;
+
+    public string BaseName { get; set; } = null!;


Suggested change

public string BaseName { get; set; } = null!;

public required string BaseName { get; set; }

Ale32bit · 2026-08-31T14:48:30Z

+
+    public int PeriodMinutes { get; set; }
+
+    public string Description { get; set; } = null!;


Suggested change

public string Description { get; set; } = null!;

public required string Description { get; set; }

Ale32bit · 2026-08-31T14:48:57Z

+{
+    public int Id { get; set; }
+
+    public string Description { get; set; } = null!;


Suggested change

public string Description { get; set; } = null!;

public required string Description { get; set; }

Ale32bit · 2026-08-31T14:56:37Z

+        }
+    }
+
+    private static string[]? NormalizeAllowedSubscribers(IEnumerable<string>? addresses)


Make it return List<string> or IEnumerable<string>.

Ale32bit · 2026-08-31T14:56:58Z

+        return normalized
+            .Distinct(StringComparer.Ordinal)
+            .OrderBy(q => q, StringComparer.Ordinal)
+            .ToArray();


Suggested change

.ToArray();

.ToList();

Ale32bit · 2026-08-31T14:58:41Z

+            NextPayment = subscription is not null &&
+                          (subscription.Status == SubscriptionStatus.Active ||
+                           (subscription.CancellationReason == ReasonUnsubscribed &&
+                            subscription.NextPayment > DateTime.UtcNow))
+                ? subscription.NextPayment
+                : null,


Extract this condition before the call for clarity.

Ale32bit · 2026-08-31T15:00:35Z

Revert changes, use the user secrets.

Ale32bit · 2026-08-31T15:02:35Z

You could consider parallelising or multithreading.

feat(kromer): implement subscriptions

a40081c

Laincy reviewed Apr 27, 2026

View reviewed changes

Laincy added the enhancement New feature or request label Apr 27, 2026

fix(kromer): refine subscription billing and cancellation

2460ed4

feat(kromer): add subscription caps, allowlists, and closing, and fix…

bde1144

… migration not being visible

bananasov requested changes May 10, 2026

View reviewed changes

feat(kromer): support multi wallet subscription queries :3

e77b2c8

qrmcat requested a review from bananasov August 30, 2026 12:20

EmmaKnijn requested review from Ale32bit, Laincy and dimaguy August 30, 2026 12:24

bananasov approved these changes Aug 31, 2026

View reviewed changes

Ale32bit requested changes Aug 31, 2026

View reviewed changes

	public string Address { get; set; } = null!;
	public required string Address { get; set; }

	public string Receiver { get; set; } = null!;
	public required string Receiver { get; set; }


		public string Receiver { get; set; } = null!;

		public string BaseName { get; set; } = null!;

	public string BaseName { get; set; } = null!;
	public required string BaseName { get; set; }


		public int PeriodMinutes { get; set; }

		public string Description { get; set; } = null!;

	public string Description { get; set; } = null!;
	public required string Description { get; set; }

Uh oh!

Conversation

qrmcat commented Apr 27, 2026

Uh oh!

Laincy left a comment

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!

Laincy commented Apr 27, 2026

Uh oh!

qrmcat commented Apr 27, 2026

Uh oh!

qrmcat commented Apr 27, 2026

Uh oh!

dimaguy commented Apr 27, 2026

Uh oh!

qrmcat commented Apr 27, 2026

Uh oh!

qrmcat commented Apr 29, 2026

Uh oh!

Twijn commented May 7, 2026

Uh oh!

bananasov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qrmcat commented Aug 30, 2026

Uh oh!

bananasov left a comment

Choose a reason for hiding this comment

Uh oh!

Ale32bit left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants