Enable bots in GDMs with sponsor-based entitlements#4860
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughEvent and protocol payloads gained an optional app_sponsor_address for memberships; constructors and snapshot writers were updated to populate it conditionally. GDM membership rules now add sponsor-based chain-auth checks and propagate the sponsor address for bot joins. Tests and call sites updated accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@core/node/rules/can_add_event.go`:
- Around line 2023-2037: gdmBotSponsorEntitlements can call
auth.NewChainAuthArgsForIsAppInstalled with a zero sponsor address when
AppSponsorAddress is empty; add a guard after computing sponsorAddress (from
ru.membership.GetAppSponsorAddress()) to return nil,nil if sponsorAddress ==
(common.Address{}) so the entitlement check only runs for bot joins with a
non-zero sponsor and non-zero appAddress (keep existing appAddress and
MembershipOp_SO_JOIN checks).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
core/node/protocol/protocol.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (5)
core/node/events/events.gocore/node/events/snapshot.gocore/node/events/stream_viewstate_space_test.gocore/node/rules/can_add_event.goprotocol/protocol.proto
🚧 Files skipped from review as they are similar to previous changes (3)
- core/node/events/stream_viewstate_space_test.go
- core/node/events/snapshot.go
- protocol/protocol.proto
🧰 Additional context used
📓 Path-based instructions (3)
core/**/*.go
📄 CodeRabbit inference engine (core/AGENTS.md)
core/**/*.go: Format all modified Go files using ./fmt.sh from the /core directory before committing
Run the linter using ./lint.sh from the /core directory to check code quality, which runs both golangci-lint and staticcheck
Use RiverError type from node/base/error.go for structured errors
Use RiverErrorWithBase to construct a river error derived from an error of arbitrary or unknown type
Wrap blockchain errors with context using crypto/ utilities
Log errors with structured fields using zap logger
Use node/crypto/blockchain.go for Ethereum client operations
Use node/crypto/chain_monitor.go for transaction monitoring
Use xchain/entitlement/ for cross-chain entitlement checks
Do not run ./fmt.sh and ./lint.sh at the same time; run them as separate commands
Use go.uber.org/zap for structured logging with custom extensions from node/logging/
Use go.opentelemetry.io for distributed tracing across the application
Use prometheus for OpenMetrics in application metrics collection
Files:
core/node/events/events.gocore/node/rules/can_add_event.go
**/core/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/core/**/*.go: Format all Go files using the fmt.sh script from the /core directory
Run the linter using the lint.sh script from the /core directory for all Go code changes
Use structured logging with zap for Go development
Implement proper error handling with RiverError types in Go code
Implement proper context cancellation for graceful shutdowns in Go code
Files:
core/node/events/events.gocore/node/rules/can_add_event.go
**/core/node/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use structured logging with zap in Go backend River nodes for observability
Files:
core/node/events/events.gocore/node/rules/can_add_event.go
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: packages/examples/bot-quickstart/AGENTS.md:0-0
Timestamp: 2026-01-09T20:07:46.072Z
Learning: Applies to packages/examples/bot-quickstart/**/*.{ts,tsx} : Ensure `bot.appAddress` (Smart Account) is funded for on-chain operations, and `bot.botId` (Gas Wallet/EOA) is the signing wallet.
📚 Learning: 2025-12-17T02:40:57.825Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: core/AGENTS.md:0-0
Timestamp: 2025-12-17T02:40:57.825Z
Learning: Applies to core/**/*.go : Use xchain/entitlement/ for cross-chain entitlement checks
Applied to files:
core/node/rules/can_add_event.go
🧬 Code graph analysis (2)
core/node/events/events.go (1)
core/node/protocol/protocol.pb.go (8)
MemberPayload_Membership(6401-6416)MemberPayload_Membership(6431-6431)MemberPayload_Membership(6446-6448)MemberPayload(1498-1517)MemberPayload(1532-1532)MemberPayload(1547-1549)MemberPayload_Membership_(1639-1641)MemberPayload_Membership_(1683-1683)
core/node/rules/can_add_event.go (3)
core/node/shared/ids.go (1)
ValidGDMChannelStreamId(277-279)core/node/events/events.go (1)
Make_MemberPayload_Membership(190-220)core/node/auth/auth_impl.go (2)
ChainAuthArgs(213-242)NewChainAuthArgsForIsAppInstalled(101-107)
🔇 Additional comments (5)
core/node/rules/can_add_event.go (3)
519-520: LGTM!Clean integration of sponsor entitlement checks for GDM bot joins while maintaining no-op for regular users.
1602-1625: LGTM!Bot sponsor validation correctly enforces that bots joining GDMs must have a sponsor set to the initiator.
1860-1876: LGTM!Correctly sets sponsor to initiator for GDM bot joins when creating derived membership events.
core/node/events/events.go (2)
190-220: LGTM!Clean API extension with conditional sponsor address assignment to avoid unnecessary payload bloat.
347-347: LGTM!Call sites correctly pass zero address for sponsor in legacy helpers.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@core/node/rpc/gdm_bot_membership_test.go`:
- Around line 181-184: The test currently ignores errors from crypto.NewWallet
when creating aliceWallet, bobWallet, carolWallet, and botWallet; update the
test to check each returned error and fail the test on error (e.g., use
t.Fatalf/t.Fatalf or require.NoError) immediately after each call to
crypto.NewWallet so wallet creation failures are surfaced rather than ignored.
- Around line 149-152: The test currently ignores errors returned by
crypto.NewWallet when creating aliceWallet, bobWallet, carolWallet, and
daveWallet; update the test to check and handle those errors (e.g., using
require.NoError or if err != nil t.Fatalf) immediately after each call to
crypto.NewWallet so failures are surfaced and subsequent code doesn't run with
nil/invalid wallets.
- Around line 117-120: The test currently ignores errors from crypto.NewWallet
when creating aliceWallet, bobWallet, carolWallet, and daveWallet; update the
code to check the returned error for each call (e.g., err :=
crypto.NewWallet(ctx)) and fail the test on error using the test helper in scope
(t.Fatal/t.Fatalf or require.NoError(t, err)) so wallet creation failures are
not silently ignored.
- Around line 86-89: The test is ignoring errors from crypto.NewWallet for
aliceWallet, bobWallet, carolWallet, and botWallet; update the test to handle
errors by either asserting no error after each call (e.g., require.NoError(t,
err) or t.Fatalf on err) or add a small helper (e.g., mustNewWallet(t, ctx) that
calls crypto.NewWallet and fails the test on error) and replace the direct calls
with that helper so any wallet creation failure fails the test immediately.
🧹 Nitpick comments (1)
core/node/rpc/gdm_bot_membership_test.go (1)
80-81: Consider addingt.Parallel()for independent tests.These tests don't share state and could run in parallel. Based on coding guidelines.
Also applies to: 111-112, 143-144, 175-176
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
core/node/rpc/gdm_bot_membership_test.go
🧰 Additional context used
📓 Path-based instructions (8)
core/node/rpc/**/*_test.go
📄 CodeRabbit inference engine (core/node/rpc/AGENTS.md)
core/node/rpc/**/*_test.go: Define service testers that wrap the baseserviceTesterto manage test lifecycle, context, cleanup, blockchain test context, River nodes, and HTTP clients
Create authenticated clients using protocolconnect service clients with the service address and HTTP client, and useauthenticateBS()to add authentication to requests
Use table-driven tests with a map structure containing test cases with expected values and error messages for comprehensive test coverage
Usesync.WaitGroupfor concurrent testing to verify thread-safe behavior and race conditions
Generate test wallets and metadata using helper functions likesafeNewWallet()andappMetadataForBot()for realistic test data
Uset.Cleanup()or defer with tester cleanup functions to handle automatic cleanup and resource management
Uset.Parallel()for tests that don't share state to enable parallel test execution
Userequireassertions for critical checks andassertfor non-critical assertions to differentiate failure severity
Pass test context through function calls for proper cancellation and lifecycle management
Enable River logs withenableRiverLogs: truein tester options when debugging test failures
Files:
core/node/rpc/gdm_bot_membership_test.go
core/node/rpc/**/*.go
📄 CodeRabbit inference engine (core/node/rpc/AGENTS.md)
core/node/rpc/**/*.go: Services run on separate listeners/ports from main River nodes and require separate service initialization with distinct authentication and configuration
Configure service-specific settings likeAppRegistryId,SharedSecretDataEncryptionKey, andAllowInsecureWebhooksin configuration before service initializationUse connectrpc.com/connect (Connect protocol) for HTTP/gRPC compatibility in service implementations
Files:
core/node/rpc/gdm_bot_membership_test.go
core/**/*.go
📄 CodeRabbit inference engine (core/AGENTS.md)
core/**/*.go: Format all modified Go files using ./fmt.sh from the /core directory before committing
Run the linter using ./lint.sh from the /core directory to check code quality, which runs both golangci-lint and staticcheck
Use RiverError type from node/base/error.go for structured errors
Use RiverErrorWithBase to construct a river error derived from an error of arbitrary or unknown type
Wrap blockchain errors with context using crypto/ utilities
Log errors with structured fields using zap logger
Use node/crypto/blockchain.go for Ethereum client operations
Use node/crypto/chain_monitor.go for transaction monitoring
Use xchain/entitlement/ for cross-chain entitlement checks
Do not run ./fmt.sh and ./lint.sh at the same time; run them as separate commands
Use go.uber.org/zap for structured logging with custom extensions from node/logging/
Use go.opentelemetry.io for distributed tracing across the application
Use prometheus for OpenMetrics in application metrics collection
Files:
core/node/rpc/gdm_bot_membership_test.go
core/**/*_test.go
📄 CodeRabbit inference engine (core/AGENTS.md)
core/**/*_test.go: Use testify/require and testify/assert for test assertions in Go test files
Database tests should use testutils/dbtestutils for test DB lifecycle management
Do not set GOCACHE or GOMODCACHE when running Go test commands; run go commands directly without setting extra environment variables
Set RIVER_TEST_LOG=info and RIVER_TEST_PRINT=1 environment variables for debug output in tests
Mock implementations should be used from testutils/mocks/ in tests
Files:
core/node/rpc/gdm_bot_membership_test.go
**/core/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/core/**/*.go: Format all Go files using the fmt.sh script from the /core directory
Run the linter using the lint.sh script from the /core directory for all Go code changes
Use structured logging with zap for Go development
Implement proper error handling with RiverError types in Go code
Implement proper context cancellation for graceful shutdowns in Go code
Files:
core/node/rpc/gdm_bot_membership_test.go
**/core/**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/core/**/*_test.go: Use testify for test assertions in Go tests
Make tests re-runnable by using unique identifiers (e.g., UUIDs) instead of hardcoded values
Files:
core/node/rpc/gdm_bot_membership_test.go
**/core/node/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use structured logging with zap in Go backend River nodes for observability
Files:
core/node/rpc/gdm_bot_membership_test.go
**/core/node/rpc/**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Service tests should validate business logic and API behavior
Files:
core/node/rpc/gdm_bot_membership_test.go
🧠 Learnings (10)
📚 Learning: 2025-11-25T08:47:01.034Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: core/node/app_registry/AGENTS.md:0-0
Timestamp: 2025-11-25T08:47:01.034Z
Learning: Applies to core/node/app_registry/**/app_registry/**/*_test.go : Create RPC test cases using NewAppRegistryServiceTester(), RegisterBotService() for test bots, and authenticateBS() for authenticated requests
Applied to files:
core/node/rpc/gdm_bot_membership_test.go
📚 Learning: 2025-11-25T08:47:37.754Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: core/node/rpc/AGENTS.md:0-0
Timestamp: 2025-11-25T08:47:37.754Z
Learning: Applies to core/node/rpc/**/*_test.go : Generate test wallets and metadata using helper functions like `safeNewWallet()` and `appMetadataForBot()` for realistic test data
Applied to files:
core/node/rpc/gdm_bot_membership_test.go
📚 Learning: 2025-11-25T08:47:37.754Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: core/node/rpc/AGENTS.md:0-0
Timestamp: 2025-11-25T08:47:37.754Z
Learning: Applies to core/node/rpc/**/*_test.go : Use `sync.WaitGroup` for concurrent testing to verify thread-safe behavior and race conditions
Applied to files:
core/node/rpc/gdm_bot_membership_test.go
📚 Learning: 2026-01-08T18:27:30.866Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T18:27:30.866Z
Learning: Applies to **/core/node/rpc/**/*_test.go : Service tests should validate business logic and API behavior
Applied to files:
core/node/rpc/gdm_bot_membership_test.go
📚 Learning: 2025-11-25T08:47:37.754Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: core/node/rpc/AGENTS.md:0-0
Timestamp: 2025-11-25T08:47:37.754Z
Learning: Applies to core/node/rpc/**/*_test.go : Use table-driven tests with a map structure containing test cases with expected values and error messages for comprehensive test coverage
Applied to files:
core/node/rpc/gdm_bot_membership_test.go
📚 Learning: 2025-11-25T08:47:25.078Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: core/node/authentication/AGENTS.md:0-0
Timestamp: 2025-11-25T08:47:25.078Z
Learning: Applies to core/node/authentication/**/*_test.go : In test code, use helper functions like authenticateBS() to add authentication to requests and test with different wallets
Applied to files:
core/node/rpc/gdm_bot_membership_test.go
📚 Learning: 2025-11-25T08:48:08.084Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: core/node/storage/AGENTS.md:0-0
Timestamp: 2025-11-25T08:48:08.084Z
Learning: Applies to core/node/storage/storage/**/*_test.go : Use standard imports in storage tests: context, testing, github.com/ethereum/go-ethereum/common, github.com/stretchr/testify/require, and relevant River protocol packages
Applied to files:
core/node/rpc/gdm_bot_membership_test.go
📚 Learning: 2025-11-25T08:47:37.754Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: core/node/rpc/AGENTS.md:0-0
Timestamp: 2025-11-25T08:47:37.754Z
Learning: Applies to core/node/rpc/**/*_test.go : Use `t.Parallel()` for tests that don't share state to enable parallel test execution
Applied to files:
core/node/rpc/gdm_bot_membership_test.go
📚 Learning: 2025-11-25T08:48:08.084Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: core/node/storage/AGENTS.md:0-0
Timestamp: 2025-11-25T08:48:08.084Z
Learning: Applies to core/node/storage/storage/**/*_test.go : Use helper functions to generate test data consistently: `safeAddress(t)` for Ethereum addresses, `testAppMetadataWithName(name string)` for app metadata, `testutils.FakeStreamId(streamType)` for stream IDs
Applied to files:
core/node/rpc/gdm_bot_membership_test.go
📚 Learning: 2025-11-25T08:47:37.754Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: core/node/rpc/AGENTS.md:0-0
Timestamp: 2025-11-25T08:47:37.754Z
Learning: Applies to core/node/rpc/**/*_test.go : Pass test context through function calls for proper cancellation and lifecycle management
Applied to files:
core/node/rpc/gdm_bot_membership_test.go
🧬 Code graph analysis (1)
core/node/rpc/gdm_bot_membership_test.go (6)
core/node/crypto/sign.go (2)
Wallet(107-111)NewWallet(113-137)core/node/protocol/protocolconnect/protocol.connect.go (1)
StreamServiceClient(100-121)core/node/shared/ids.go (1)
UserStreamIdFromAddr(136-141)core/node/shared/stream_id.go (2)
StreamId(41-41)STREAM_GDM_CHANNEL_BIN(20-20)core/node/events/events.go (2)
MakeEnvelopeWithPayload(146-152)Make_UserPayload_Membership(578-601)core/node/protocol/protocol.pb.go (1)
MembershipOp_SO_JOIN(89-89)
🔇 Additional comments (6)
core/node/rpc/gdm_bot_membership_test.go (6)
19-46: LGTM!Helper correctly creates a bot user stream with app_address in inception.
48-78: LGTM!Helper correctly creates self-signed join events. The comment on line 52 could be clearer - the inviter is always set in the payload but ignored server-side when the event is user-signed rather than node-signed.
92-109: LGTM!Test correctly validates bot can join GDM with sponsor mechanism.
137-141: LGTM!Good negative test case for unauthorized self-join.
169-173: LGTM!Important edge case - validates inviter field is ignored for user-signed events.
201-233: LGTM!Good test validating that direct membership events to GDM streams are rejected - ensures proper flow through user stream.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
This comment has been minimized.
This comment has been minimized.
3 participants
Summary
app_sponsor_addressfield toMemberPayload.MembershipandMemberPayload.Snapshot.Memberapp_sponsor_addressequalsinitiator_addressfor bot GDM joins