Conversation
|
| var.PROVER_OXIDE_SIDECAR.enabled ? {} : { | ||
| "node.node.env.PROVER_NODE_DISABLE_PROOF_PUBLISH" = var.PROVER_NODE_DISABLE_PROOF_PUBLISH |
There was a problem hiding this comment.
Proof publishing override When the sidecar is enabled with
PROVER_NODE_DISABLE_PROOF_PUBLISH=true, as configured for mainnet, this branch drops the disable setting. The prover ConfigMap then sets it to false and Terraform supplies a signer, so the node can submit L1 proof transactions despite the explicit no-publish setting. Reject that combination or preserve the setting.
Knowledge Base Used: Prover node orchestration
| RPC_GATEWAY_KONG_INGRESS_CLASS=testnet-infra-rpc-kong | ||
| PROVER_NODE_RPC_GATEWAY_HOSTS='["prover.testnet.rpc.aztec-labs.com"]' | ||
| PROVER_NODE_RPC_GATEWAY_API_KEY_SECRET_NAMES='["testnet-prover-rpc-consumer-client1"]' | ||
| PROVER_NODE_RPC_GATEWAY_ENABLED=false |
There was a problem hiding this comment.
Authenticated prover route removed With
RPC_GATEWAY_ENABLED=false, disabling the prover gateway also removes testnet’s authenticated prover.testnet.rpc.aztec-labs.com route and its API-key consumer. If clients use that endpoint, they lose access; the new sidecar connects through a pod-local address and does not replace their route. Preserve an access path or provide a migration before removing it.
Knowledge Base Used: Operations and deployment
| # Derivation for publisher addresses when using web3signer | ||
| PUBLISHERS_PER_PROVER: "1" | ||
| PUBLISHER_KEY_INDEX_START: "8000" | ||
| # -- Optional epoch-proof worker; see oxide-relayer.md for activation requirements. |
There was a problem hiding this comment.
Missing activation guide This comment directs operators to
oxide-relayer.md, but that document is not present in the repository. Operators cannot find the activation requirements. The repository requires comment references to be understandable from the repository alone, so add the guide or state the requirements here before merging.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| VALIDATOR_RESOURCE_PROFILE="prod" | ||
| VALIDATOR_COINBASE="0x36502A83735ED62671B55858809703898cDE4f95" | ||
|
|
||
| PROVER_ID_MNEMONIC_INDEX=7999 |
There was a problem hiding this comment.
testnet.env sets the prover ID mnemonic index to 7999 while the sidecar stays disabled:
PROVER_ID_MNEMONIC_INDEX=7999
PROVER_OXIDE_SIDECAR='{"enabled": false}'Terraform turns this into KEY_INDEX_START = coalesce(7999, 8000) (main.tf:477). With no sidecar there is no /shared/prover-id, so setup-prover-keystore.sh derives the prover id at 7999. At base it was 8000. The keystore id wins over the env value (prover-node/src/factory.ts:86), so the next testnet deploy proves under a new address, and rewards and anything keyed on the prover address move with it.
Only testnet-v5 needs 7999, because the relayer signs with that key. Can you drop this line from testnet.env, or say in the PR description that you are staging the new id on purpose?
Written by Claude
| RPC_GATEWAY_KONG_INGRESS_CLASS=testnet-infra-rpc-kong | ||
| PROVER_NODE_RPC_GATEWAY_HOSTS='["prover.testnet.rpc.aztec-labs.com"]' | ||
| PROVER_NODE_RPC_GATEWAY_API_KEY_SECRET_NAMES='["testnet-prover-rpc-consumer-client1"]' | ||
| PROVER_NODE_RPC_GATEWAY_ENABLED=false |
There was a problem hiding this comment.
This turns off the prover RPC gateway on testnet. That removes the host prover.testnet.rpc.aztec-labs.com and its consumer testnet-prover-rpc-consumer-client1. RPC_GATEWAY_ENABLED is already false, so kong_gateway_enabled also goes false (main.tf:119) and the whole testnet Kong release is removed (main.tf:889).
The sidecar stays off on testnet, so nothing replaces that access, and the sidecar work does not need this change. I can't tell from the repo whether this is a problem: the consumer may already have moved, or the removal may be intended. The PR does not mention it. Is it intentional? If so, a line in the description (or a separate PR) would help.
Written by Claude
| initContainers: '{{ include "prover.oxideSidecar.initContainer" . }}' | ||
| extraVolumes: '{{ include "prover.oxideSidecar.volumes" . }}' |
There was a problem hiding this comment.
The sidecar's init container and volumes live in node.initContainers and node.extraVolumes. Helm users set those same values for their own additions, and list-form initContainers already worked at base.
A user who sets their own init container (a snapshot restore, say) and enables the sidecar replaces derive-prover-identity. Render and validation pass and the relayer starts, then oxide-wait.mjs:6 fails with ENOENT on /oxide-identity/key. Overriding extraVolumes leaves the mounts without volumes, and the API server rejects the pod. No env file or Terraform in the repo sets these keys, so this hits direct Helm users only.
Can you give the sidecar its own slots that the pod template composes with the user's lists? Failing validation when the user overrides these values would also work.
Written by Claude
| elif [ -n "${PROVER_ID:-}" ]; then | ||
| address=$PROVER_ID |
There was a problem hiding this comment.
The new branch copies PROVER_ID into the keystore:
elif [ -n "${PROVER_ID:-}" ]; then
address=$PROVER_IDPROVER_ID is set only from node.coinbase (_pod-template.yaml:383). At base the keystore always carried the mnemonic-derived id, and the keystore id beats the env var, so a prover with node.coinbase set now proves under a different id. The sidecar never reaches this branch, since the /shared/prover-id check comes first, and no prover in the repo sets coinbase, so only outside chart users are affected. Unless you have a use for it, drop these two lines.
Written by Claude
| wait.mjs: | | ||
| {{- .Files.Get "scripts/oxide-wait.mjs" | nindent 4 }} | ||
| derive-prover-identity.sh: | | ||
| {{- .Files.Get "scripts/derive-prover-identity.sh" | nindent 4 }} |
There was a problem hiding this comment.
wait.mjs and derive-prover-identity.sh reach the pod through this parent-chart ConfigMap. The pod template's checksum annotations cover only the subchart's scripts ConfigMap, the env secret and .Values (aztec-node _pod-template.yaml:13-18), not this ConfigMap.
So a later change to either script alone updates the ConfigMap but does not roll the StatefulSet. The init container's old output stays and the relayer keeps the old startup hook. The deploy succeeds, but the change takes effect only after an unrelated restart. Please fix this in this PR rather than later.
Put a checksum of these two files on the pod template. node.podAnnotations is rendered with plain toYaml today, so one way is to run it through the same chart.renderExtension/tpl helper and have the prover stack set checksum/oxide-startup from the two files.
Written by Claude
7531fe7 to
4b625f3
Compare
Fixes A-2048