add modifier to EntitlementChecker to only allow registered operator to be checkers #80
Conversation
There was a problem hiding this comment.
At a v high level, this looks fine to me - but I don't know what's going on in a lot of / most of this PR, so I'm not sure I'm the best person to approve it. If you can't get someone else, let me know if you want to jump on a call.
request for changes for a nit.
@clemire The main component that's either missing or I'm not able to asses is whether or not we're registering operators prior to calling cc. @brianathere |
| @@ -53,7 +53,7 @@ contract EntitlementChecker is IEntitlementChecker, Facet { | |||
| * @param node The address of the node to register | |||
| * @dev Only valid operators can register a node | |||
| */ | |||
| function registerNode(address node) external { | |||
| function registerNode(address node) external onlyRegisteredOperator { | |||
There was a problem hiding this comment.
You'll need to change files like core/node/crypto/chain_txpool_test.go to work with this modifier. Specifically lines like:
return tc.NodeRegistry.RegisterNode(opts, nodeWallet.Address, url, 2)
need to use the operator wallet instead of the deployer wallet:
txPool, err := crypto.NewTransactionPoolWithPolicies(
ctx, tc.Client(), tc.DeployerBlockchain.Wallet, resubmitPolicy, repricePolicy, tc.ChainMonitor)
require.NoError(err, "unable to construct transaction pool")
core/xchain/create_multi.sh
Outdated
| @@ -59,6 +59,8 @@ BASE_DIR="./run_files/${RUN_ENV}" | |||
|
|
|||
| mkdir -p "${BASE_DIR}" | |||
|
|
|||
| source ../../contracts/.env.localhost | |||
There was a problem hiding this comment.
@brianathere I made the following changes to get xchain to properly register itself on startup, to mimic how stream node registers when the service is being launched (using the same wallet as the operator address), but was unsuccessful.
There was a problem hiding this comment.
Right now xchain isn't launching, and it's failing on the registration step. This is why the sdk tests for space admission that require xchain in the loop are failing.
|
I have added 2 steps that first register a node operator and that operator than registeres the xchain nodes. Not sure if this is the best place for it because running the script a second time will now give an error because it tries to register an operator/node for the second time. For CI that isn't an issue, for development it can be annoying. |
core/xchain/launch_multi.sh
Outdated
| BASE_DIR="${RUN_FILES_DIR}/${RUN_ENV}" | ||
| BASE_REGISTRY_ADDRESS=$(jq -r '.address' ../../packages/generated/deployments/local_single/base/addresses/baseRegistry.json) | ||
| OPERATOR_ADDRESS=$(cast wallet addr $LOCAL_PRIVATE_KEY) |
There was a problem hiding this comment.
you can't hard code local_single here - you need to respect the RUN_ENV
There was a problem hiding this comment.
just change this to
BASE_REGISTRY_ADDRESS=$(jq -r '.address' ../../packages/generated/deployments/${RIVER_ENV}/base/addresses/baseRegistry.json)
but it doesn't seem to fix it
yes, can you move these calls to the create_multi.sh script and fix the hard coded local_single (change to ${RIVER_ENV})? |
Actually there are several things that need to happen for this to work, i will update this pr |
Testnet Proposal: https://testnets.llama.xyz/orgs/river/base-sepolia/actions/100
Mainnet Proposal: https://app.llama.xyz/orgs/river/base/actions/34