[API-291] Migrate /comms/mutate to bridge #337

schottra · 2025-08-19T20:58:07Z

Our story begins with this endpoint implementation in the protocol comms server...

In order to properly handle a mutation request, we need:

The validator logic, including all the DB queries used to verify the various conditions
The RPCProcessor, including the utility queries used to write various data
The rate limiter helper used by the validator/processor
Some config constants

And then I modified it with:

Changed all query patterns from sqlx to pgx patterns
Passing request context through instead of using background since this is called as part of a network request
Instead of using hefty shared queries in the validator, updated it to use lighter "exists" style queries that just validate a condition we expect instead of returning whole rows of data for us to parse through in code
Removed all code related to forwarding and receiving rpcLog messages between servers. However we are still inserting them and checking for duplicates in the cases where users double submit. Note: I did change the 'relayed by' to a static string since it's required in the DB model. Bonus this will help us differentiate between requests handled by bridge and those handled by legacy DN.

To be handled in a follow-up:

Re-enabling websocket broadcast

raymondjacobson

TBH I only skimmed the sql stuff, it seems generally right. But no harm in getting this in, I see!

raymondjacobson · 2025-08-21T01:07:05Z

api/comms/chat.go

+	ChatPermissionFollowees,
+	ChatPermissionFollowers,
+	ChatPermissionTippees,
+	ChatPermissionTippers,


do we have permission for coins yet? maybe not

raymondjacobson · 2025-08-21T01:10:02Z

api/comms/chat.go

+					)
+				)
+		)
+		OR from_user_id IN (


oh I see we have it here

raymondjacobson · 2025-08-21T01:10:46Z

api/comms/chat.go

+}
+
+// Helper function to get new blasts as potential chat seeds for creating a chat
+func getNewBlasts(tx pgx.Tx, ctx context.Context, arg getNewBlastsParams) ([]blastRow, error) {


I actually rather like this function

it's a great fn

raymondjacobson · 2025-08-21T01:18:16Z

api/comms/signed_request.go

+
+	wallet := crypto.PubkeyToAddress(*pubkey).Hex()
+
+	// TODO: Still need this? We have a function for getting these in another file


This is fine to remove. All we should need is this

recoverPubkeyFromCoreTx

which gets called from

https://github.com/AudiusProject/audius-protocol/blob/930bbe8a229322b13546b6023771d7334119179d/comms/discovery/pubkeystore/backfill.go#L21

We should probably do this in band rather than in a recovery process on a timer though. It would make things much more stable. cc @dharit-tan too

i dont rly remember the context for this missing pubkey business

so basically when we first launched DMs the shared secret computation relied on the full public key for the wallet (which is more than just the account address that is stored in our wallet field on user).

in order to get that full public key, we need to derive it from prior POA -> ACDC -> core transactions.

So there was a big thing about doing backfill, etc. etc., which is now fully done.

Going forward, we just need to recover this from core. But even better would just be to store it when we create the user and index that, entirely outside of the DMs flow

@raymondjacobson if I understand you correctly, we don't need to add keys to a store anymore, but we do need to move the "recover from core" functionality over to this comms implementation?

We probably do want to add them to the store, just not necessarily as a polling job. I think there are other consumers

config/config.go

dharit-tan · 2025-08-21T01:46:43Z

api/comms/rpc_processor.go

+					logger.Error("err: invalid json", zap.Error(err))
+				} else {
+					// TODO
+					// websocketNotify(json.RawMessage(j), userId, messageTs.Round(time.Microsecond))


plz dont flip prod over to this until we have this sorted! will break chat blasts

Oh yeah don't sweat, there are at least two additional PRs to finish migrating functionality before I'm even going to turn this on in staging 😅

dharit-tan · 2025-08-21T01:50:23Z

api/comms/validator.go

+
+	// This query checks for the existence of a new blast from a specific user
+	// using the same logic as GetNewBlasts but optimized for existence check
+	var hasNewBlast = `


feel like this sql query should be shared with getNewBlasts. maybe can turn it into a sql function? don't want them to get out of sync.

I had a similar thought. We have been following a convention of not sharing queries in bridge (other than the core entity fetches) because it gets us into situations where editing one endpoint breaks other endpoints.

We also don't want to return any data from this query, since it's just validating existence before we try to apply things. I will play around with merging things in my next PR and see if I can get it to a spot that feels good. Otherwise, options are:

SQL function per your suggestion

Change how createChat works and require client to pass a blast message id if we're trying to initialize from a blast, instead of having the server try and figure out if it should be a blast response or not. Not sure if this is feasible, but it would greatly simplify things here because we wouldn't need the huge queries to try and find a blast that applies to this user.

option 2 would mean:

client fetches list of pending blasts (on user login)

then passes that list back to the server in createChat
technically possible but gets tricky if there are many pending blasts, eg. if the user hasn't logged in in a long time

tbh would prefer not to change this pattern

Oh hmm, my assumption was that createChat would be called after we had already fetched all the pending blasts anyway (i.e. they are opening the conversation for that blast, and then potentially responding to it). If that's not always the case, then I agree with you that it would be a pain on the client to fetch all of them first.

oh yeah you're right sry, we do call createChat for each blast. however the chat could have more than one blast to seed the chat with (ie blast sender has sent many blasts in a row). then it's kinda hard for the client to be sure that it fetched all the pending blasts, and also group them into their respective chats. easier to just do on the backend i think?
relevant sdk code: https://github.com/AudiusProject/audius-protocol/blob/912a2eafa3cf730cbe7c436bd3b016a401af0687/packages/sdk/src/sdk/api/chats/ChatsApi.ts#L805

looking at the sdk it even seems like we don't paginate thru blasts (sus lol). but we do not group the blasts, we just grab each unique chatId that could be created from each of the blasts and then call create on that. tbh i think what ur saying is possible, i just think that it's easier to have a shared sql on the backend to handle this. d to discuss tho. but also like if it aint broke...

Definitely seems like it's more complicated than I thought. Not trying to change it right this second! Thanks for indulging.

dharit-tan · 2025-08-21T01:50:50Z

trashid/chatid.go

@@ -14,3 +15,8 @@ func ChatID(id1, id2 int) string {
 	}
 	return chatId
 }
+
+// Returns a unique Message ID for a blast message in a chat.
+func BlastMessageID(blastID, chatID string) string {


dharit-tan

i think the chat blasts sql query should be shared but other than that lgtm

schottra added 4 commits August 19, 2025 12:47

checkpoint for comms mutations, validator linting clean

fdb5438

migrate rpc processor

e2e982d

finish wiring up mutate endpoint

7cad355

cleanup

22e89b7

schottra requested review from raymondjacobson and rickyrombo August 19, 2025 21:05

raymondjacobson approved these changes Aug 21, 2025

View reviewed changes

dharit-tan reviewed Aug 21, 2025

View reviewed changes

dharit-tan approved these changes Aug 21, 2025

View reviewed changes

schottra added 3 commits August 21, 2025 10:15

some fixes

604f59f

PR feedback and cleanup

246840b

temp test hacks

e03357a

schottra force-pushed the turtle-power branch from f84eb11 to e03357a Compare August 21, 2025 14:44

schottra merged commit 28808be into main Aug 21, 2025
5 checks passed

schottra deleted the turtle-power branch August 21, 2025 14:48


		wallet := crypto.PubkeyToAddress(*pubkey).Hex()

		// TODO: Still need this? We have a function for getting these in another file

[API-291] Migrate /comms/mutate to bridge #337

[API-291] Migrate /comms/mutate to bridge #337

Uh oh!

Conversation

schottra commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

raymondjacobson 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!

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!

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!

dharit-tan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

schottra commented Aug 19, 2025 •

edited

Loading