Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate channel READ/WRITE entitlements locally on stream node #19

Conversation

clemire
Copy link
Contributor

@clemire clemire commented May 21, 2024

For READ and WRITE permissions on the stream node, the stream node now fetches entitlements (including xchain entitlements), evaluates them, and caches them in positive / negative cache depending on result. This includes a ban check. All other permissions are checked via the usual route.

Note: channel and space entitlements evaluated on the stream node are now evaluated through the same flow:

  1. if owner, universally pass
  2. if banned,reject
  3. otherwise, evaluate entitlements

Other changes:

  • unified wallet linking logic between xchain and stream node

@clemire clemire changed the base branch from main to crystal/hnt-6476-get-channel-based-entitlements-for-permission May 21, 2024 00:29
@clemire clemire changed the base branch from crystal/hnt-6476-get-channel-based-entitlements-for-permission to main May 21, 2024 00:30
@clemire clemire changed the title Crystal/hnt 6492 fetch channel permissions from chain and evaluate locally on Evaluate channel READ/WRITE entitlements locally on stream node May 23, 2024
@@ -103,4 +103,6 @@ network:
# Debug feature flags.
disableBaseChain: <DISABLE_BASE_CHAIN>

chains: '31337:http://localhost:8545,31338:http://localhost:8546,84532:https://sepolia.base.org,11155111:https://ethereum-sepolia-rpc.publicnode.com'
Copy link
Contributor Author

@clemire clemire May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed for local and CI testing to be successful, it configures the stream node to use these as asset chains for entitlement checks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason for using external blockchains for local tests?

Copy link
Contributor Author

@clemire clemire Jun 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anvil doesn't behave the same way as external chains, which has exposed us to issues in the past because xchain incompatibilities weren't caught until deploying. However I'm not sure we're testing external chains on stream node, this is just the same config as what xchain is using for local testing, which I think makes sense - we still have two node config template files at this time so it has to be copied.

}

// Sanity checks
log := dlog.FromCtx(ctx).With("function", "evaluateCheckOperation")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explicit errors instead of segfaults with these checks

const createAliceAndBobStart = Date.now()
// This test is commented out as the membership joinSpace does not check linked wallets
// against the user entitlement.
test.skip('userEntitlementPass - join as root, linked wallet whitelisted', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case checked the wrong condition - I fixed and commented out. Turns out wallet linking is not implemented in the space contract for space joins, will need to fix this one and below.

@clemire clemire requested a review from brianathere May 31, 2024 22:44
return &banning{
contract: contract,
spaceAddress: spaceAddress,
bannedAddressCache: NewBannedAddressCache(time.Duration(cfg.NegativeEntitlementCacheTTLSeconds) * time.Second),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value is never set in config and is always 0.

Default value used in other parts of the code is 2 seconds.

@clemire clemire merged commit 72696b4 into main Jun 4, 2024
9 checks passed
@clemire clemire deleted the crystal/hnt-6492-fetch-channel-permissions-from-chain-and-evaluate-locally-on branch June 4, 2024 17:30
clemire added a commit that referenced this pull request Jun 5, 2024
clemire added a commit that referenced this pull request Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants