feat(storage-evm): EnergyPPAv2 contracts, VPP module, and Base mainnet demo#2190
Open
n0umen0n wants to merge 36 commits into
Open
feat(storage-evm): EnergyPPAv2 contracts, VPP module, and Base mainnet demo#2190n0umen0n wants to merge 36 commits into
n0umen0n wants to merge 36 commits into
Conversation
Contributor
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (12)
WalkthroughIntroduces the Hypha Energy Community feature: new ChangesEVM Contracts, VPP Module, Scripts, Tests, and Docs
Web Energy Feature (API, UI, DB, CI)
Sequence Diagram(s)sequenceDiagram
participant Browser
participant SpaceEnergyRoute as GET /api/v1/spaces/[spaceSlug]/energy
participant DB as Postgres DB
participant Factory as EnergyPPAv2Factory (on-chain)
participant Proxy as EnergyPPAv2 Proxy (on-chain)
Browser->>SpaceEnergyRoute: request
SpaceEnergyRoute->>DB: findSpaceBySlug
SpaceEnergyRoute->>DB: findEnergyCommunityBySpaceId
alt no DB mapping
SpaceEnergyRoute->>Factory: getAdminCommunities(adminCandidate)
SpaceEnergyRoute->>Factory: communities(latestCommunityId)
SpaceEnergyRoute->>DB: upsertEnergyCommunityActivation
end
SpaceEnergyRoute->>Proxy: safeRead (fees, balances, sourceIds, members, roles, sources)
SpaceEnergyRoute-->>Browser: enabled, activation, overview, sources, members, roles, optimization
sequenceDiagram
participant UserEnergySection
participant useUserEnergy
participant PeopleEnergyRoute as GET /api/v1/people/[personSlug]/energy
participant DB as Postgres DB
participant OnChain as EnergyPPAv2 Proxies (on-chain)
UserEnergySection->>useUserEnergy: render
useUserEnergy->>PeopleEnergyRoute: fetch (Bearer token)
PeopleEnergyRoute->>DB: findPersonByToken + findSpacesByIds
PeopleEnergyRoute->>OnChain: getMemberSpaces, getCreditBalance, getDebt, getCredit, getOwnerships
PeopleEnergyRoute-->>useUserEnergy: communities[], totals
useUserEnergy-->>UserEnergySection: data
UserEnergySection->>OnChain: settleOwnDebt / claimCredit (on button click)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 25
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/storage-evm/contracts/EnergyPPAv2.sol`:
- Around line 693-710: _getEnergyCreditBalance currently returns a positive
energyToken.balanceOf(account) ignoring any negative
energyCreditBalances[account], and verifyZeroSum only iterates registered
members risking invariant breaks when tokens are transferable; fix by treating
the true account balance as net = int256(energyToken.balanceOf(account)) +
energyCreditBalances[account] (use this in _getEnergyCreditBalance and all
checks), and ensure _setEnergyCreditBalance updates both energyToken and
energyCreditBalances consistently (burn/transfer must adjust token and keep
energyCreditBalances as the signed remainder), and either make energyToken
non-transferable or extend verifyZeroSum to include all token holders (or
maintain a registry updated on transfers) so the zero-sum invariant covers
transferred tokens.
- Around line 282-288: The export branch currently allows deactivated sources to
accrue revenue; in the block that handles exports (where r.deviceId ==
exportDeviceId) add the same active-source guard used in the local-consumption
path by requiring sources[r.sourceId].active before computing revenue (in
addition to the existing require against IMPORT_SOURCE_ID), so
_addSourceRevenue(srcRevenues, r.sourceId, ...) and emit EnergyExported only run
for active sources.
- Around line 633-674: The issue: operational backend addresses in onlyWhitelist
have admin powers (can call setEnergyToken, setStablecoin, emergencyReset,
setPaymentRecipient, setCommunityAddress, setAggregatorAddress,
setCommunityFeeBps, setAggregatorFeeBps, setGridOperator) and can thus corrupt
accounting; fix by separating roles so admin-only config functions use a
stronger modifier (e.g., onlyOwner or a new onlyAdmin) while leaving
onlyWhitelist for consume/operational functions; update the modifiers on the
listed functions (setEnergyToken, setStablecoin, emergencyReset,
setPaymentRecipient, setCommunityAddress, setAggregatorAddress,
setCommunityFeeBps, setAggregatorFeeBps, setGridOperator) to require owner/admin
and adjust any access control storage/initializer if you introduce a new admin
role.
- Around line 365-389: The code incorrectly uses token.totalSupply() as the
denominator while only iterating memberAddresses, causing any supply held by
non-members to be implicitly paid to the last member; fix by computing the sum
of balances for memberAddresses (e.g., sumMemberBalances) and use that sum
instead of token.totalSupply() when calculating each member's share and the
remainder logic (update the variables used in the loop such as
token.totalSupply(), memberAddresses, lastHolder, remaining, and distributed);
if sumMemberBalances == 0 return early to avoid distributing to non-members.
- Around line 433-453: The removeMember function currently deletes member state
and burns positive token balance without ensuring the member has zero net
obligations, which can break the contract's zero-sum invariant; update
removeMember to require that the member has no outstanding net balance before
removal by checking members[memberAddress].settledBalance == 0,
energyCreditBalances[memberAddress] == 0, and
energyToken.balanceOf(memberAddress) == 0 (or alternatively force-settle/zero
those counters and update any counterparties before deletion), and only then
proceed to clear deviceToMember, memberAddresses, delete members and
energyCreditBalances and call energyToken.burn; reference symbols: removeMember,
members[memberAddress].settledBalance, energyCreditBalances,
energyToken.balanceOf, energyToken.burn, deviceToMember.
- Around line 488-492: The contract EnergyPPAv2.sol currently uses raw
IERC20.transfer and transferFrom calls on the local variable coin which is
brittle for non-standard ERC20s; import OpenZeppelin's SafeERC20, add "using
SafeERC20 for IERC20;" to the contract, and replace all raw
coin.transferFrom(...) and coin.transfer(...) calls (the occurrences around the
settlement/claim logic where IERC20 coin = IERC20(stablecoinAddress) is used)
with coin.safeTransferFrom(...) and coin.safeTransfer(...) respectively,
removing the surrounding require(...) boolean checks so transfers use
SafeERC20's safe wrappers.
In `@packages/storage-evm/contracts/EnergyPPAv2Factory.sol`:
- Around line 117-118: Do not transfer EnergyToken ownership to p.admin because
that lets the admin call EnergyToken.setAuthorized and grant arbitrary minters;
instead wire ownership/minter rights to the deployed PPA proxy or an explicit
minter-only mechanism: remove or replace energyToken.transferOwnership(p.admin)
with energyToken.transferOwnership(proxy) so the proxy (not p.admin) is owner,
or implement an immutable/minter-only path in EnergyToken (e.g., add an
immutable address minter or setMinter(address) restricted to constructor/owner)
and assign that minter to the proxy, keeping energyToken.setAuthorized(proxy,
true) as the only mint/burn authority for the PPA proxy. Ensure changes
reference EnergyToken, energyToken.setAuthorized, energyToken.transferOwnership,
proxy, and p.admin.
In `@packages/storage-evm/contracts/EnergyToken.sol`:
- Around line 7-10: Update the NatSpec for the EnergyToken contract to
consistently reflect that this ERC20 represents positive energy credit balances
and ties mint/burn authority to the new EnergyPPAv2 contract (not
EnergyDistribution), and remove any references to USDC; specifically, edit the
top contract comment for EnergyToken and the later documentation block that
currently mentions USDC so both say "positive energy credit balances" and state
"Only the EnergyPPAv2 contract can mint and burn tokens" (reference: contract
EnergyToken and any NatSpec blocks referencing mint/burn authority).
In `@packages/storage-evm/README-energy-demo.md`:
- Line 66: Several fenced code blocks in the README use unlabeled backticks
(e.g., the block that starts with "Members:", the narrative lines like
"Households consume energy …", "Debtors pay in (settleDebt) …", and the formula
"sum(all member credits) + community + aggregator − gridBalance + settledBalance
= 0"); label each of these triple-backtick fences with the language identifier
text (i.e., change ``` to ```text) so markdownlint rule MD040 is satisfied and
diagrams/formulas are correctly classified.
In
`@packages/storage-evm/scripts/base-mainnet-contracts-scripts/deploy-energy-ppa-v2-factory.ts`:
- Around line 228-240: The deployCommunity call includes gridOperator but it
isn’t documented or persisted; update the file header “Community parameters” to
list gridOperator, add a gridOperator field to the DeploymentState
type/structure (the same place DeploymentState is defined) and ensure the value
(adminAddress) is written into addresses.json when deployment state is saved,
and optionally mention/verify usage in downstream demo scripts like
energy-ppav2-vpp-loop.ts which call ppa.getGridOperator() so the persisted
addresses include the grid operator entry.
- Around line 376-400: Add an env override check for ENERGY_PPAV2_IMPL and reuse
it instead of always deploying: read getEnv('ENERGY_PPAV2_IMPL') before the
existingFactory branch, and if set assign energyPPAv2Impl to that value and log
reuse (same pattern used for existingFactory/REGULAR_SPACE_TOKEN_IMPL); only
call ethers.getContractFactory('EnergyPPAv2') and deploy when neither
ENERGY_PPAV2_IMPL nor existingFactory is provided, keeping the variable name
energyPPAv2Impl and the rest of the flow unchanged so the script remains
idempotent.
In
`@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts`:
- Around line 384-487: The planInterval function duplicates allocation logic —
replace it by calling the VPP module's fairSplit and buildConsumptionReadings to
reuse the canonical allocator: invoke fairSplit(...) with the same inputs
(solarProduction, bat1Available, bat2Available, hhDemands or equivalent) and
then call buildConsumptionReadings(...) to produce the readings, then return an
IntervalPlan-shaped object that includes solarProduction, bat1Available,
bat2Available, hhDemands, totals and the values returned from fairSplit
(solarUsed, bat1Used, bat2Used, importQty, exportQty) plus the readings from
buildConsumptionReadings; remove the inline allocation code in planInterval and
import fairSplit and buildConsumptionReadings from packages/storage-evm/vpp
(matching how energy-ppav2-vpp-loop.ts does).
- Around line 431-432: The demand-rebalancing currently does: compute total =
hhSolar+hhBat1+hhBat2+hhImport and, if total !== demand, it increments hhSolar
by demand-total which can over-assign solar (hhSolar) beyond available solarUsed
and misattribute energy to consumeEnergy/verifyZeroSum; change the rebalancing
so that if total < demand you route the residual to IMPORT (increment hhImport)
or, alternatively, cap hhSolar per household so the sum of all hhSolar values
never exceeds solarUsed before adjusting hhImport; update the logic that adjusts
hhSolar/hhImport around variables hhSolar, hhImport, demand, importQty and
ensure per-source totals stay bounded relative to solarUsed and are used when
calling consumeEnergy/verifyZeroSum.
- Around line 320-341: The catch and log-parsing code use TypeScript any which
loses type-safety: change the catch parameter in the try/catch around the
staticCall and any other catches to use unknown and narrow it (e.g., const err =
e as Error when checking message/reason) and update the receipt.log mapping to
use proper types (use ethers.LogDescription | null for parsed entries) and add a
safe type-guard when calling factory.interface.parseLog so parse results are
typed; target the try/catch that logs "staticCall REVERTED" and the block that
builds event from receipt.logs and the variables factory, receipt, and event to
ensure no any usage remains.
- Line 492: The local variable declaration "ppa: any" should be replaced with a
concrete contract type obtained from ethers.getContractAt or your
TypeChain-generated interface (e.g., use the PPAV2/PPAv2Contract type returned
by getContractAt), and all catch parameters typed as `unknown` instead of `any`
(e.g., change catch (e: any) to catch (e: unknown) and then narrow to Error or
stringify for logging). Locate the usages of ppa (variable name "ppa") and the
catch blocks with parameter "e" and update their types accordingly, ensuring you
import the TypeChain type for the contract or use ReturnType<typeof
ethers.getContractAt> to type ppa and use type guards when handling errors.
In
`@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-vpp-loop.ts`:
- Around line 962-972: The infinite for(;;) loop around runBatch currently
swallows persistent errors and only sleeps a fixed loopMs; add a consecutive
failure counter (e.g., consecutiveFailures) that increments in the catch block
and resets to 0 on successful runBatch, compute an exponential backoff delay =
min(loopMs * 2**consecutiveFailures, maxDelayMs) before awaiting sleep(), and
log the backoff delay and full error details; also add a maxFailures kill-switch
(e.g., if consecutiveFailures >= maxFailures then process.exit(1) or throw) to
stop the loop on repeated unrecoverable failures. Ensure you update references
to runBatch, batch, loopMs, and sleep in the loop so the new logic is applied.
- Around line 459-463: The function printOnChainState and other places use
untyped any for the contract parameter (ppa) and caught errors (e); replace
ppa:any with a concrete contract type from your TypeChain artifacts or the
inferred return type of ethers.getContractAt (e.g., MyPPAContract or
ReturnType<typeof ethers.getContractAt>) and change caught errors from any to
unknown and narrow-check them (e.g., instanceof Error) before accessing
message/stack; update all occurrences referenced (printOnChainState, other
functions at the noted locations) to use these proper types and adjust
signatures and error handling accordingly.
- Around line 941-950: The reset branch currently calls ppa.emergencyReset()
directly when ENERGY_DEMO_COMMAND==='reset', which is dangerous on mainnet; add
a confirmation guard: require an explicit environment flag (e.g. CONFIRM_RESET
=== 'yes') or verify the network (e.g. block if chainId === 8453) and print the
target ppa proxy address before proceeding; if the guard is not satisfied, log a
clear abort message and return without calling emergencyReset(); reference the
existing symbols ppa, emergencyReset, printOnChainState and the
ENERGY_DEMO_COMMAND/CONFIRM_RESET env variables when implementing this check.
In `@packages/storage-evm/test/EnergyPPAv2Functional.test.ts`:
- Around line 198-199: The test is calling removed getters
getImportEnergyCreditBalance() and getExportEnergyCreditBalance() on the
EnergyPPAv2 instance (ppa) which no longer exist; replace those calls with the
new consolidated getGridBalance() and update downstream assertions/uses
accordingly (e.g., if split import/export values were expected, adapt to the
single returned structure or destructure fields from getGridBalance()). Locate
usages in the test around the references to ppa.getImportEnergyCreditBalance and
ppa.getExportEnergyCreditBalance and change them to ppa.getGridBalance(), then
adjust any variable names (importBal/exportBal) and assertions to match the
shape and semantics returned by getGridBalance() in the EnergyPPAv2 contract.
- Around line 90-99: The deployProxy call is missing the new gridOperator
initializer argument for EnergyPPAv2.initialize (which expects five args);
update the upgrades.deployProxy invocation that creates ppa so the initializer
args array includes the gridOperator contract/address (e.g., add
gridOperator.target or gridOperator.address as the fifth element) so the call
matches EnergyPPAv2.initialize(owner.address, energyToken.target,
stablecoin.target, ethers.ZeroAddress, gridOperator).
- Around line 1150-1202: The test is failing because the CommunityParams struct
passed to factory.deployCommunity is missing the required gridOperator field;
update the object passed to factory.deployCommunity to include gridOperator
(e.g., gridOperator: owner.address or another valid signer/address) so the
CommunityParams struct encoding matches the contract signature used by
deployCommunity.
In `@packages/storage-evm/vpp/build-readings.ts`:
- Around line 93-96: scaleQuantity currently drops up to scale-1 Wh per reading
by using Math.floor(wh / scale), which causes cumulative "dust" loss when
quantityScale is large; either document this accounting impact in the function
JSDoc (mention quantityScale and that on-chain verifyZeroSum still holds but VPP
fair-split totals may differ) or change the scaling strategy to conserve total
energy (e.g., compute integer-scaled values and distribute the remainder across
readings so the lastReading or a deterministic set of readings are incremented
to absorb the leftover). Update scaleQuantity (and callers such as
energy-ppav2-vpp-loop.ts) accordingly so the logic and JSDoc reflect the chosen
approach.
In `@packages/storage-evm/vpp/on-chain-reader.ts`:
- Around line 72-87: Filter out inactive sources by checking src.active after
calling ppa.getSource(sourceId) and only include sources where active is true;
when mapping sourceType use SOURCE_TYPE_MAP[Number(src.sourceType)] but if that
lookup is undefined throw an error (fail fast) instead of defaulting to 'SOLAR';
build and return activeSources (the filtered array) and continue to call
readOwnershipBps for the retained sources only; update references to the created
array (e.g., sources → activeSources) so downstream code uses the filtered,
validated list.
- Around line 52-60: The code is converting uint256 device IDs to JS Number
(e.g., exportDeviceId = Number(exportDeviceIdBn) and (m.deviceIds as
bigint[]).map(Number)), which can lose precision; change the types and logic to
preserve bigint: update MemberInfo.deviceIds to bigint[],
OnChainConfig.exportDeviceId to bigint, and OnChainConfig.deviceToMember to use
bigint keys, then remove Number(...) conversions in on-chain-reader.ts (around
exportDeviceId and the member mapping where ppa.getMember is processed) so you
keep device IDs as bigint throughout the pipeline and only convert to Number
where you intentionally and safely need to (with explicit guards).
In `@packages/storage-evm/vpp/run-interval.ts`:
- Around line 67-93: parseIntervalReadings currently accumulates consumption for
any member resolved via deviceToMember but input.members passed to fairSplit is
filtered to only active members, causing unpaired consumption and invariant
failures in fairSplit; fix by ensuring consumption keys match active members:
either (a) change parseIntervalReadings or a post-processing step to filter the
returned consumption object to only include addresses present in
members.filter(m=>m.isActive).map(m=>m.address), or (b) detect when a reading
maps to an inactive member and throw a clear error. Update the code paths around
parseIntervalReadings, the creation of input (members/consumption), and any
callers of fairSplit so that totalConsumption only includes active members (or
fails fast if inactive-member readings exist) before calling fairSplit and
buildConsumptionReadings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce6ceaa3-37c1-4955-9617-6deaa04fe726
📒 Files selected for processing (17)
packages/storage-evm/README-energy-demo.mdpackages/storage-evm/contracts/EnergyPPAv2.solpackages/storage-evm/contracts/EnergyPPAv2Factory.solpackages/storage-evm/contracts/EnergyToken.solpackages/storage-evm/scripts/base-mainnet-contracts-scripts/deploy-energy-ppa-v2-factory.tspackages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-demo-state.jsonpackages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.tspackages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-vpp-loop.tspackages/storage-evm/test/EnergyPPAv2Functional.test.tspackages/storage-evm/test/EnergyPPAv2Gas.test.tspackages/storage-evm/vpp/build-readings.tspackages/storage-evm/vpp/fair-split.test.tspackages/storage-evm/vpp/fair-split.tspackages/storage-evm/vpp/index.tspackages/storage-evm/vpp/on-chain-reader.tspackages/storage-evm/vpp/run-interval.tspackages/storage-evm/vpp/types.ts
Comment on lines
+282
to
+288
| // ── Export ── | ||
| if (r.deviceId == exportDeviceId) { | ||
| require(r.sourceId != IMPORT_SOURCE_ID, 'Cannot export import'); | ||
| uint256 revenue = r.quantity * r.pricePerKwh; | ||
| _addSourceRevenue(srcRevenues, r.sourceId, revenue); | ||
| gridBalance += int256(revenue); | ||
| emit EnergyExported(r.quantity, revenue, r.sourceId); |
Contributor
There was a problem hiding this comment.
Reject exports for inactive sources.
The local-consumption path checks sources[r.sourceId].active, but the export path only checks r.sourceId != IMPORT_SOURCE_ID. A deactivated source can still accrue export revenue and be distributed.
✅ Proposed active-source check
if (r.deviceId == exportDeviceId) {
require(r.sourceId != IMPORT_SOURCE_ID, 'Cannot export import');
+ require(sources[r.sourceId].active, 'Source not active');
uint256 revenue = r.quantity * r.pricePerKwh;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // ── Export ── | |
| if (r.deviceId == exportDeviceId) { | |
| require(r.sourceId != IMPORT_SOURCE_ID, 'Cannot export import'); | |
| uint256 revenue = r.quantity * r.pricePerKwh; | |
| _addSourceRevenue(srcRevenues, r.sourceId, revenue); | |
| gridBalance += int256(revenue); | |
| emit EnergyExported(r.quantity, revenue, r.sourceId); | |
| // ── Export ── | |
| if (r.deviceId == exportDeviceId) { | |
| require(r.sourceId != IMPORT_SOURCE_ID, 'Cannot export import'); | |
| require(sources[r.sourceId].active, 'Source not active'); | |
| uint256 revenue = r.quantity * r.pricePerKwh; | |
| _addSourceRevenue(srcRevenues, r.sourceId, revenue); | |
| gridBalance += int256(revenue); | |
| emit EnergyExported(r.quantity, revenue, r.sourceId); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/contracts/EnergyPPAv2.sol` around lines 282 - 288, The
export branch currently allows deactivated sources to accrue revenue; in the
block that handles exports (where r.deviceId == exportDeviceId) add the same
active-source guard used in the local-consumption path by requiring
sources[r.sourceId].active before computing revenue (in addition to the existing
require against IMPORT_SOURCE_ID), so _addSourceRevenue(srcRevenues, r.sourceId,
...) and emit EnergyExported only run for active sources.
Comment on lines
+365
to
+389
| address tokenAddr = sources[sourceId].ownershipToken; | ||
| IERC20 token = IERC20(tokenAddr); | ||
| uint256 totalSupply = token.totalSupply(); | ||
| if (totalSupply == 0) return; | ||
|
|
||
| address lastHolder = address(0); | ||
| for (uint256 i = 0; i < memberAddresses.length; i++) { | ||
| if (token.balanceOf(memberAddresses[i]) > 0) { | ||
| lastHolder = memberAddresses[i]; | ||
| } | ||
| } | ||
| if (lastHolder == address(0)) return; | ||
|
|
||
| uint256 distributed = 0; | ||
| for (uint256 i = 0; i < memberAddresses.length; i++) { | ||
| address addr = memberAddresses[i]; | ||
| uint256 tokenBal = token.balanceOf(addr); | ||
| if (tokenBal == 0) continue; | ||
|
|
||
| uint256 share; | ||
| if (addr == lastHolder) { | ||
| share = remaining - distributed; | ||
| } else { | ||
| share = (remaining * tokenBal) / totalSupply; | ||
| } |
Contributor
There was a problem hiding this comment.
Do not distribute non-member ownership shares to the last registered member.
Revenue is split using token.totalSupply(), but only memberAddresses are enumerated. If ownership tokens are held by a non-member or transferred out, their proportional share is not paid to them; the final member with a non-zero balance receives the remainder.
🧾 Suggested guard until holder enumeration exists
address tokenAddr = sources[sourceId].ownershipToken;
IERC20 token = IERC20(tokenAddr);
uint256 totalSupply = token.totalSupply();
if (totalSupply == 0) return;
+
+ uint256 trackedSupply = 0;
+ for (uint256 i = 0; i < memberAddresses.length; i++) {
+ trackedSupply += token.balanceOf(memberAddresses[i]);
+ }
+ require(trackedSupply == totalSupply, 'Untracked source token holders');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| address tokenAddr = sources[sourceId].ownershipToken; | |
| IERC20 token = IERC20(tokenAddr); | |
| uint256 totalSupply = token.totalSupply(); | |
| if (totalSupply == 0) return; | |
| address lastHolder = address(0); | |
| for (uint256 i = 0; i < memberAddresses.length; i++) { | |
| if (token.balanceOf(memberAddresses[i]) > 0) { | |
| lastHolder = memberAddresses[i]; | |
| } | |
| } | |
| if (lastHolder == address(0)) return; | |
| uint256 distributed = 0; | |
| for (uint256 i = 0; i < memberAddresses.length; i++) { | |
| address addr = memberAddresses[i]; | |
| uint256 tokenBal = token.balanceOf(addr); | |
| if (tokenBal == 0) continue; | |
| uint256 share; | |
| if (addr == lastHolder) { | |
| share = remaining - distributed; | |
| } else { | |
| share = (remaining * tokenBal) / totalSupply; | |
| } | |
| address tokenAddr = sources[sourceId].ownershipToken; | |
| IERC20 token = IERC20(tokenAddr); | |
| uint256 totalSupply = token.totalSupply(); | |
| if (totalSupply == 0) return; | |
| uint256 trackedSupply = 0; | |
| for (uint256 i = 0; i < memberAddresses.length; i++) { | |
| trackedSupply += token.balanceOf(memberAddresses[i]); | |
| } | |
| require(trackedSupply == totalSupply, 'Untracked source token holders'); | |
| address lastHolder = address(0); | |
| for (uint256 i = 0; i < memberAddresses.length; i++) { | |
| if (token.balanceOf(memberAddresses[i]) > 0) { | |
| lastHolder = memberAddresses[i]; | |
| } | |
| } | |
| if (lastHolder == address(0)) return; | |
| uint256 distributed = 0; | |
| for (uint256 i = 0; i < memberAddresses.length; i++) { | |
| address addr = memberAddresses[i]; | |
| uint256 tokenBal = token.balanceOf(addr); | |
| if (tokenBal == 0) continue; | |
| uint256 share; | |
| if (addr == lastHolder) { | |
| share = remaining - distributed; | |
| } else { | |
| share = (remaining * tokenBal) / totalSupply; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/contracts/EnergyPPAv2.sol` around lines 365 - 389, The
code incorrectly uses token.totalSupply() as the denominator while only
iterating memberAddresses, causing any supply held by non-members to be
implicitly paid to the last member; fix by computing the sum of balances for
memberAddresses (e.g., sumMemberBalances) and use that sum instead of
token.totalSupply() when calculating each member's share and the remainder logic
(update the variables used in the loop such as token.totalSupply(),
memberAddresses, lastHolder, remaining, and distributed); if sumMemberBalances
== 0 return early to avoid distributing to non-members.
Comment on lines
+433
to
+453
| function removeMember(address memberAddress) external onlyWhitelist { | ||
| require(members[memberAddress].isActive, 'Member does not exist'); | ||
| Member memory m = members[memberAddress]; | ||
|
|
||
| for (uint256 i = 0; i < m.deviceIds.length; i++) { | ||
| delete deviceToMember[m.deviceIds[i]]; | ||
| } | ||
|
|
||
| for (uint256 i = 0; i < memberAddresses.length; i++) { | ||
| if (memberAddresses[i] == memberAddress) { | ||
| memberAddresses[i] = memberAddresses[memberAddresses.length - 1]; | ||
| memberAddresses.pop(); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| delete members[memberAddress]; | ||
| delete energyCreditBalances[memberAddress]; | ||
|
|
||
| uint256 tokenBal = energyToken.balanceOf(memberAddress); | ||
| if (tokenBal > 0) energyToken.burn(memberAddress, tokenBal); |
Contributor
There was a problem hiding this comment.
Require a zero balance before removing a member.
removeMember deletes signed debt/credit and burns positive credit tokens without adjusting settledBalance or any counterparty balance. Removing a member with a non-zero balance permanently breaks the zero-sum invariant.
🛡️ Proposed invariant-preserving guard
function removeMember(address memberAddress) external onlyWhitelist {
require(members[memberAddress].isActive, 'Member does not exist');
+ require(
+ _getEnergyCreditBalance(memberAddress) == 0,
+ 'Member balance must be settled'
+ );
Member memory m = members[memberAddress];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/contracts/EnergyPPAv2.sol` around lines 433 - 453, The
removeMember function currently deletes member state and burns positive token
balance without ensuring the member has zero net obligations, which can break
the contract's zero-sum invariant; update removeMember to require that the
member has no outstanding net balance before removal by checking
members[memberAddress].settledBalance == 0, energyCreditBalances[memberAddress]
== 0, and energyToken.balanceOf(memberAddress) == 0 (or alternatively
force-settle/zero those counters and update any counterparties before deletion),
and only then proceed to clear deviceToMember, memberAddresses, delete members
and energyCreditBalances and call energyToken.burn; reference symbols:
removeMember, members[memberAddress].settledBalance, energyCreditBalances,
energyToken.balanceOf, energyToken.burn, deviceToMember.
Comment on lines
+488
to
+492
| IERC20 coin = IERC20(stablecoinAddress); | ||
| require( | ||
| coin.transferFrom(msg.sender, address(this), requiredStablecoin), | ||
| 'Stablecoin transfer failed' | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm OpenZeppelin SafeERC20 import path exists in the installed dependency tree.
fd SafeERC20.sol . | sed -n '1,20p'Repository: hypha-dao/hypha-web
Length of output: 45
🏁 Script executed:
# Check if the file exists
ls -la packages/storage-evm/contracts/EnergyPPAv2.solRepository: hypha-dao/hypha-web
Length of output: 159
🏁 Script executed:
# Check the OpenZeppelin package version and SafeERC20 availability
cat packages/storage-evm/package.json | grep -A 5 -B 5 "@openzeppelin"Repository: hypha-dao/hypha-web
Length of output: 45
🏁 Script executed:
# Check SafeERC20 availability in node_modules
find packages/storage-evm/node_modules -name "SafeERC20.sol" 2>/dev/null | head -5Repository: hypha-dao/hypha-web
Length of output: 45
🏁 Script executed:
# Read the EnergyPPAv2.sol file to inspect the mentioned lines
wc -l packages/storage-evm/contracts/EnergyPPAv2.solRepository: hypha-dao/hypha-web
Length of output: 113
🏁 Script executed:
# Read the beginning of EnergyPPAv2.sol to check imports
head -50 packages/storage-evm/contracts/EnergyPPAv2.solRepository: hypha-dao/hypha-web
Length of output: 2467
🏁 Script executed:
# Read the specific lines mentioned in the review
sed -n '488,492p' packages/storage-evm/contracts/EnergyPPAv2.sol
sed -n '542,545p' packages/storage-evm/contracts/EnergyPPAv2.sol
sed -n '569,573p' packages/storage-evm/contracts/EnergyPPAv2.sol
sed -n '612,615p' packages/storage-evm/contracts/EnergyPPAv2.solRepository: hypha-dao/hypha-web
Length of output: 620
🏁 Script executed:
# Find package.json in storage-evm directory
cat packages/storage-evm/package.json | jq '.devDependencies, .dependencies' | grep -i "openzeppelin"Repository: hypha-dao/hypha-web
Length of output: 45
🏁 Script executed:
# Read package.json directly
cat packages/storage-evm/package.jsonRepository: hypha-dao/hypha-web
Length of output: 721
🏁 Script executed:
# Check OpenZeppelin installation and SafeERC20 structure
ls -la packages/storage-evm/node_modules/@openzeppelin/contracts/token/ERC20/utils/ 2>/dev/null | head -20Repository: hypha-dao/hypha-web
Length of output: 45
🏁 Script executed:
# Check if OpenZeppelin is installed in node_modules
ls -la packages/storage-evm/node_modules/@openzeppelin/ 2>/dev/nullRepository: hypha-dao/hypha-web
Length of output: 45
🏁 Script executed:
# Check root package.json for OpenZeppelin
cat package.json | grep -A 3 -B 3 "@openzeppelin"Repository: hypha-dao/hypha-web
Length of output: 662
🏁 Script executed:
# Search for `@openzeppelin` in the entire workspace
find . -maxdepth 3 -name "package.json" -exec grep -l "@openzeppelin" {} \;Repository: hypha-dao/hypha-web
Length of output: 77
🏁 Script executed:
# Verify SafeERC20.sol exists in OpenZeppelin 5.2.0 structure
ls -la node_modules/@openzeppelin/contracts/token/ERC20/utils/ 2>/dev/null | grep SafeERC20Repository: hypha-dao/hypha-web
Length of output: 126
🏁 Script executed:
# Check the content of SafeERC20 to confirm it exists
head -20 node_modules/@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol 2>/dev/nullRepository: hypha-dao/hypha-web
Length of output: 842
🏁 Script executed:
# Confirm the exact import path needed
grep -r "SafeERC20" node_modules/@openzeppelin/contracts/ | grep "^.*SafeERC20.sol:" | head -1Repository: hypha-dao/hypha-web
Length of output: 213
🏁 Script executed:
# Check if SafeERC20 is already imported or used anywhere in EnergyPPAv2.sol
grep -n "SafeERC20" packages/storage-evm/contracts/EnergyPPAv2.solRepository: hypha-dao/hypha-web
Length of output: 45
🏁 Script executed:
# Check if SafeERC20 is used in other contracts in the same directory
grep -r "SafeERC20" packages/storage-evm/contracts/Repository: hypha-dao/hypha-web
Length of output: 1537
🏁 Script executed:
# Get full context around the problematic lines to understand the functions
sed -n '480,500p' packages/storage-evm/contracts/EnergyPPAv2.sol
sed -n '535,555p' packages/storage-evm/contracts/EnergyPPAv2.sol
sed -n '562,580p' packages/storage-evm/contracts/EnergyPPAv2.sol
sed -n '605,625p' packages/storage-evm/contracts/EnergyPPAv2.solRepository: hypha-dao/hypha-web
Length of output: 2575
Use SafeERC20 for stablecoin transfers.
The stablecoin is configurable, but raw IERC20.transfer/transferFrom is brittle for non-standard ERC-20s that don't return boolean values. Use OpenZeppelin SafeERC20 consistently for settlement and claims, matching the pattern already adopted in other contracts in this directory (EscrowImplementation.sol, RegularSpaceToken.sol, TokenBackingVaultImplementation.sol).
Add the import:
import '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol';Add the using statement to the contract:
using SafeERC20 for IERC20;Then replace the four occurrences (lines 488–492, 542–545, 569–573, 612–615) of raw transferFrom and transfer calls with their safe equivalents: coin.safeTransferFrom(...) and coin.safeTransfer(...) without the require wrappers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/contracts/EnergyPPAv2.sol` around lines 488 - 492, The
contract EnergyPPAv2.sol currently uses raw IERC20.transfer and transferFrom
calls on the local variable coin which is brittle for non-standard ERC20s;
import OpenZeppelin's SafeERC20, add "using SafeERC20 for IERC20;" to the
contract, and replace all raw coin.transferFrom(...) and coin.transfer(...)
calls (the occurrences around the settlement/claim logic where IERC20 coin =
IERC20(stablecoinAddress) is used) with coin.safeTransferFrom(...) and
coin.safeTransfer(...) respectively, removing the surrounding require(...)
boolean checks so transfers use SafeERC20's safe wrappers.
Comment on lines
+633
to
+674
| function setEnergyToken(address tokenAddress) external onlyWhitelist { | ||
| require(tokenAddress != address(0), 'Invalid address'); | ||
| energyToken = EnergyToken(tokenAddress); | ||
| } | ||
|
|
||
| function setStablecoin(address tokenAddress) external onlyWhitelist { | ||
| stablecoinAddress = tokenAddress; | ||
| } | ||
|
|
||
| function setExportDeviceId(uint256 deviceId) external onlyWhitelist { | ||
| exportDeviceId = deviceId; | ||
| emit ExportDeviceIdSet(deviceId); | ||
| } | ||
|
|
||
| function setCommunityAddress(address addr) external onlyOwner { | ||
| communityAddress = addr; | ||
| } | ||
|
|
||
| function setAggregatorAddress(address addr) external onlyOwner { | ||
| aggregatorAddress = addr; | ||
| } | ||
|
|
||
| function setCommunityFeeBps(uint16 bps) external onlyOwner { | ||
| require(bps + aggregatorFeeBps <= 10000, 'Total fees exceed 100%'); | ||
| communityFeeBps = bps; | ||
| } | ||
|
|
||
| function setAggregatorFeeBps(uint16 bps) external onlyOwner { | ||
| require(communityFeeBps + bps <= 10000, 'Total fees exceed 100%'); | ||
| aggregatorFeeBps = bps; | ||
| } | ||
|
|
||
| function setPaymentRecipient(address recipient) external onlyOwner { | ||
| paymentRecipient = recipient; | ||
| } | ||
|
|
||
| function setGridOperator(address operator) external onlyOwner { | ||
| gridOperatorAddress = operator; | ||
| emit GridOperatorSet(operator); | ||
| } | ||
|
|
||
| function emergencyReset() external onlyWhitelist { |
Contributor
There was a problem hiding this comment.
Separate settlement operators from admin configuration powers.
Any whitelisted backend needed for consumeEnergy can also replace the credit token/stablecoin and call emergencyReset. That conflates operational permissions with admin authority and can corrupt accounting or halt settlement.
🔐 Minimal role tightening
- function setEnergyToken(address tokenAddress) external onlyWhitelist {
+ function setEnergyToken(address tokenAddress) external onlyOwner {
require(tokenAddress != address(0), 'Invalid address');
energyToken = EnergyToken(tokenAddress);
}
- function setStablecoin(address tokenAddress) external onlyWhitelist {
+ function setStablecoin(address tokenAddress) external onlyOwner {
stablecoinAddress = tokenAddress;
}
...
- function emergencyReset() external onlyWhitelist {
+ function emergencyReset() external onlyOwner {As per coding guidelines, contracts must be checked for access control vulnerabilities.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/contracts/EnergyPPAv2.sol` around lines 633 - 674, The
issue: operational backend addresses in onlyWhitelist have admin powers (can
call setEnergyToken, setStablecoin, emergencyReset, setPaymentRecipient,
setCommunityAddress, setAggregatorAddress, setCommunityFeeBps,
setAggregatorFeeBps, setGridOperator) and can thus corrupt accounting; fix by
separating roles so admin-only config functions use a stronger modifier (e.g.,
onlyOwner or a new onlyAdmin) while leaving onlyWhitelist for
consume/operational functions; update the modifiers on the listed functions
(setEnergyToken, setStablecoin, emergencyReset, setPaymentRecipient,
setCommunityAddress, setAggregatorAddress, setCommunityFeeBps,
setAggregatorFeeBps, setGridOperator) to require owner/admin and adjust any
access control storage/initializer if you introduce a new admin role.
Comment on lines
+1150
to
+1202
| const tx = await factory.deployCommunity({ | ||
| admin: owner.address, | ||
| stablecoin: stablecoin.target, | ||
| communityAddress: community.address, | ||
| aggregatorAddress: aggregator.address, | ||
| communityFeeBps: 500, | ||
| aggregatorFeeBps: 300, | ||
| exportDeviceId: 9999, | ||
| energyTokenName: 'Community Energy', | ||
| energyTokenSymbol: 'CET', | ||
| sources: [ | ||
| { | ||
| sourceId: solarId, | ||
| sourceType: 0, | ||
| tokenName: 'Solar', | ||
| tokenSymbol: 'SOL', | ||
| basePricePerKwh: 10, | ||
| holders: [alice.address, bob.address, charlie.address], | ||
| holderAmounts: [4000, 3500, 2500], | ||
| }, | ||
| { | ||
| sourceId: batId, | ||
| sourceType: 1, | ||
| tokenName: 'Battery', | ||
| tokenSymbol: 'BAT', | ||
| basePricePerKwh: 15, | ||
| holders: [alice.address, dave.address], | ||
| holderAmounts: [5000, 5000], | ||
| }, | ||
| ], | ||
| members: [ | ||
| { | ||
| memberAddress: alice.address, | ||
| deviceIds: [1001], | ||
| metadataHash: ethers.ZeroHash, | ||
| }, | ||
| { | ||
| memberAddress: bob.address, | ||
| deviceIds: [2001], | ||
| metadataHash: ethers.ZeroHash, | ||
| }, | ||
| { | ||
| memberAddress: charlie.address, | ||
| deviceIds: [3001], | ||
| metadataHash: ethers.ZeroHash, | ||
| }, | ||
| { | ||
| memberAddress: dave.address, | ||
| deviceIds: [4001], | ||
| metadataHash: ethers.ZeroHash, | ||
| }, | ||
| ], | ||
| }); |
Contributor
There was a problem hiding this comment.
Include gridOperator in factory deployment params.
CommunityParams includes gridOperator, but this test object omits it. The factory call will fail struct encoding or deploy with an incomplete parameter object.
✅ Proposed factory test fix
communityAddress: community.address,
aggregatorAddress: aggregator.address,
+ gridOperator: owner.address,
communityFeeBps: 500,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const tx = await factory.deployCommunity({ | |
| admin: owner.address, | |
| stablecoin: stablecoin.target, | |
| communityAddress: community.address, | |
| aggregatorAddress: aggregator.address, | |
| communityFeeBps: 500, | |
| aggregatorFeeBps: 300, | |
| exportDeviceId: 9999, | |
| energyTokenName: 'Community Energy', | |
| energyTokenSymbol: 'CET', | |
| sources: [ | |
| { | |
| sourceId: solarId, | |
| sourceType: 0, | |
| tokenName: 'Solar', | |
| tokenSymbol: 'SOL', | |
| basePricePerKwh: 10, | |
| holders: [alice.address, bob.address, charlie.address], | |
| holderAmounts: [4000, 3500, 2500], | |
| }, | |
| { | |
| sourceId: batId, | |
| sourceType: 1, | |
| tokenName: 'Battery', | |
| tokenSymbol: 'BAT', | |
| basePricePerKwh: 15, | |
| holders: [alice.address, dave.address], | |
| holderAmounts: [5000, 5000], | |
| }, | |
| ], | |
| members: [ | |
| { | |
| memberAddress: alice.address, | |
| deviceIds: [1001], | |
| metadataHash: ethers.ZeroHash, | |
| }, | |
| { | |
| memberAddress: bob.address, | |
| deviceIds: [2001], | |
| metadataHash: ethers.ZeroHash, | |
| }, | |
| { | |
| memberAddress: charlie.address, | |
| deviceIds: [3001], | |
| metadataHash: ethers.ZeroHash, | |
| }, | |
| { | |
| memberAddress: dave.address, | |
| deviceIds: [4001], | |
| metadataHash: ethers.ZeroHash, | |
| }, | |
| ], | |
| }); | |
| const tx = await factory.deployCommunity({ | |
| admin: owner.address, | |
| stablecoin: stablecoin.target, | |
| communityAddress: community.address, | |
| aggregatorAddress: aggregator.address, | |
| gridOperator: owner.address, | |
| communityFeeBps: 500, | |
| aggregatorFeeBps: 300, | |
| exportDeviceId: 9999, | |
| energyTokenName: 'Community Energy', | |
| energyTokenSymbol: 'CET', | |
| sources: [ | |
| { | |
| sourceId: solarId, | |
| sourceType: 0, | |
| tokenName: 'Solar', | |
| tokenSymbol: 'SOL', | |
| basePricePerKwh: 10, | |
| holders: [alice.address, bob.address, charlie.address], | |
| holderAmounts: [4000, 3500, 2500], | |
| }, | |
| { | |
| sourceId: batId, | |
| sourceType: 1, | |
| tokenName: 'Battery', | |
| tokenSymbol: 'BAT', | |
| basePricePerKwh: 15, | |
| holders: [alice.address, dave.address], | |
| holderAmounts: [5000, 5000], | |
| }, | |
| ], | |
| members: [ | |
| { | |
| memberAddress: alice.address, | |
| deviceIds: [1001], | |
| metadataHash: ethers.ZeroHash, | |
| }, | |
| { | |
| memberAddress: bob.address, | |
| deviceIds: [2001], | |
| metadataHash: ethers.ZeroHash, | |
| }, | |
| { | |
| memberAddress: charlie.address, | |
| deviceIds: [3001], | |
| metadataHash: ethers.ZeroHash, | |
| }, | |
| { | |
| memberAddress: dave.address, | |
| deviceIds: [4001], | |
| metadataHash: ethers.ZeroHash, | |
| }, | |
| ], | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/test/EnergyPPAv2Functional.test.ts` around lines 1150 -
1202, The test is failing because the CommunityParams struct passed to
factory.deployCommunity is missing the required gridOperator field; update the
object passed to factory.deployCommunity to include gridOperator (e.g.,
gridOperator: owner.address or another valid signer/address) so the
CommunityParams struct encoding matches the contract signature used by
deployCommunity.
Comment on lines
+93
to
+96
| function scaleQuantity(wh: number, scale: number): bigint { | ||
| if (scale === 1) return BigInt(wh); | ||
| return BigInt(Math.floor(wh / scale)); | ||
| } |
Contributor
There was a problem hiding this comment.
scaleQuantity silently drops sub-scale dust — document accounting impact.
Math.floor(wh / scale) discards up to scale-1 Wh per reading. With quantityScale=1000 (used by energy-ppav2-vpp-loop.ts) and e.g. 20 readings per batch, up to ~20 kWh × price can vanish between the fair-split result and the on-chain charges. The contract's verifyZeroSum() is still satisfied on-chain (it only sees the scaled quantities), but the VPP-computed fair-split totals won't exactly match what's settled. Worth mentioning in the JSDoc, or rounding up the last reading to conserve total Wh.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/vpp/build-readings.ts` around lines 93 - 96,
scaleQuantity currently drops up to scale-1 Wh per reading by using
Math.floor(wh / scale), which causes cumulative "dust" loss when quantityScale
is large; either document this accounting impact in the function JSDoc (mention
quantityScale and that on-chain verifyZeroSum still holds but VPP fair-split
totals may differ) or change the scaling strategy to conserve total energy
(e.g., compute integer-scaled values and distribute the remainder across
readings so the lastReading or a deterministic set of readings are incremented
to absorb the leftover). Update scaleQuantity (and callers such as
energy-ppav2-vpp-loop.ts) accordingly so the logic and JSDoc reflect the chosen
approach.
Comment on lines
+52
to
+60
| const exportDeviceId = Number(exportDeviceIdBn); | ||
|
|
||
| const members: MemberInfo[] = await Promise.all( | ||
| memberAddresses.map(async (addr) => { | ||
| const m = await ppa.getMember(addr); | ||
| return { | ||
| address: m.memberAddress as string, | ||
| deviceIds: (m.deviceIds as bigint[]).map(Number), | ||
| isActive: m.isActive as boolean, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find number-based deviceId declarations and consumers in the VPP module.
rg -n -C3 '\bdeviceId\b|\bdeviceIds\b|exportDeviceId' --iglob '*.ts'Repository: hypha-dao/hypha-web
Length of output: 50376
🏁 Script executed:
# Find the smart contract ABI and search for device ID constraints
fd -e sol . packages/storage-evm | head -20Repository: hypha-dao/hypha-web
Length of output: 1218
🏁 Script executed:
# Look for the EnergyPPA contract and check for device ID limits
rg -n 'uint256.*deviceId|deviceIds.*uint256' --glob '*.sol' -A 2 -B 2Repository: hypha-dao/hypha-web
Length of output: 11578
🏁 Script executed:
# Check if there are any comments in the contract about device ID limits
rg -n 'deviceId|MAX.*DEVICE|DEVICE.*LIMIT' --glob '*.sol' -i -A 1 -B 1Repository: hypha-dao/hypha-web
Length of output: 16227
🏁 Script executed:
# Look at the actual smart contract definitions
fd -name '*.sol' packages/storage-evm/contracts | head -10Repository: hypha-dao/hypha-web
Length of output: 293
Guard uint256-to-number conversions for device IDs; migrate type system to bigint.
Number(...) silently loses precision above Number.MAX_SAFE_INTEGER (9,007,199,254,740,991). The smart contract stores device IDs as uint256 with no upper-bound validation, making it possible for an on-chain device ID to exceed the safe integer threshold. When this happens, the VPP's routing logic will misroute readings to the wrong device/member due to precision loss.
The type system already uses bigint for the final ConsumptionReading.deviceId output, indicating bigint is intended throughout. The intermediate conversion to number in on-chain-reader.ts (lines 52, 59) and re-conversion back to bigint in build-readings.ts is unnecessary and lossy.
🛡️ Proposed migration to bigint
Update type definitions to use bigint:
MemberInfo.deviceIds: bigint[]OnChainConfig.exportDeviceId: bigintOnChainConfig.deviceToMember: Map<bigint, string>
Then remove Number() conversions entirely and keep device IDs as bigint throughout the pipeline.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/vpp/on-chain-reader.ts` around lines 52 - 60, The code
is converting uint256 device IDs to JS Number (e.g., exportDeviceId =
Number(exportDeviceIdBn) and (m.deviceIds as bigint[]).map(Number)), which can
lose precision; change the types and logic to preserve bigint: update
MemberInfo.deviceIds to bigint[], OnChainConfig.exportDeviceId to bigint, and
OnChainConfig.deviceToMember to use bigint keys, then remove Number(...)
conversions in on-chain-reader.ts (around exportDeviceId and the member mapping
where ppa.getMember is processed) so you keep device IDs as bigint throughout
the pipeline and only convert to Number where you intentionally and safely need
to (with explicit guards).
Comment on lines
+72
to
+87
| const sources: SourceInfo[] = await Promise.all( | ||
| sourceIds.map(async (sourceId) => { | ||
| const src = await ppa.getSource(sourceId); | ||
| const ownershipToken = src.ownershipToken as string; | ||
| const ownershipBps = await readOwnershipBps( | ||
| provider, | ||
| ownershipToken, | ||
| memberAddresses, | ||
| ); | ||
| return { | ||
| sourceId, | ||
| sourceType: SOURCE_TYPE_MAP[Number(src.sourceType)] ?? 'SOLAR', | ||
| basePricePerKwh: src.basePricePerKwh as bigint, | ||
| ownershipToken, | ||
| ownershipBps, | ||
| }; |
Contributor
There was a problem hiding this comment.
Skip inactive sources and fail on unknown source types.
getSource returns active, but inactive sources are still included in the VPP config. That can generate readings which consumeEnergy rejects. Also, defaulting unknown enum values to SOLAR silently misclassifies future source types.
✅ Proposed source filtering
const sources: SourceInfo[] = await Promise.all(
sourceIds.map(async (sourceId) => {
const src = await ppa.getSource(sourceId);
+ if (!(src.active as boolean)) return undefined;
+ const sourceType = SOURCE_TYPE_MAP[Number(src.sourceType)];
+ if (!sourceType) {
+ throw new Error(`Unsupported source type ${src.sourceType} for ${sourceId}`);
+ }
const ownershipToken = src.ownershipToken as string;
...
return {
sourceId,
- sourceType: SOURCE_TYPE_MAP[Number(src.sourceType)] ?? 'SOLAR',
+ sourceType,
basePricePerKwh: src.basePricePerKwh as bigint,
ownershipToken,
ownershipBps,
};
}),
);
+ const activeSources = sources.filter((s): s is SourceInfo => s !== undefined);Then return activeSources.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/vpp/on-chain-reader.ts` around lines 72 - 87, Filter out
inactive sources by checking src.active after calling ppa.getSource(sourceId)
and only include sources where active is true; when mapping sourceType use
SOURCE_TYPE_MAP[Number(src.sourceType)] but if that lookup is undefined throw an
error (fail fast) instead of defaulting to 'SOLAR'; build and return
activeSources (the filtered array) and continue to call readOwnershipBps for the
retained sources only; update references to the created array (e.g., sources →
activeSources) so downstream code uses the filtered, validated list.
Comment on lines
+67
to
+93
| const { consumption, production } = parseIntervalReadings( | ||
| intervalReadings, | ||
| onChainConfig, | ||
| vppConfig, | ||
| ); | ||
|
|
||
| const input: FairSplitInput = { | ||
| members: members.filter((m) => m.isActive).map((m) => m.address), | ||
| consumption, | ||
| production, | ||
| sources, | ||
| gridImportPrice: gridPrices.importPricePerKwh, | ||
| gridExportPrice: gridPrices.exportPricePerKwh, | ||
| }; | ||
|
|
||
| const result = fairSplit(input); | ||
|
|
||
| return buildConsumptionReadings( | ||
| result, | ||
| sources, | ||
| members, | ||
| exportDeviceId, | ||
| gridPrices.importPricePerKwh, | ||
| gridPrices.exportPricePerKwh, | ||
| quantityScale, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Inactive members' consumption is silently dropped.
parseIntervalReadings (line 67) populates consumption for any member resolved via deviceToMember, regardless of isActive. Then input.members is filtered to active-only (line 74), so fairSplit iterates only active members and silently ignores consumption for inactive ones. Energy/production balance invariants inside fairSplit (see fair-split.ts:202-209) will fail if an inactive member contributed consumption, since their Wh stays in totalConsumption but is never allocated.
Either filter consumption to active members or explicitly throw when a reading routes to an inactive member.
🛠️ Proposed fix (one approach)
case 'consumption': {
const memberAddr = onChainConfig.deviceToMember.get(reading.meter_id);
if (!memberAddr) {
throw new Error(
`Consumption meter ${reading.meter_id} not mapped to any member`,
);
}
+ const memberInfo = onChainConfig.members.find(
+ (m) => m.address === memberAddr,
+ );
+ if (!memberInfo?.isActive) {
+ throw new Error(
+ `Meter ${reading.meter_id} is routed to inactive member ${memberAddr}`,
+ );
+ }
consumption.set(
memberAddr,
(consumption.get(memberAddr) ?? 0) + reading.energy_wh,
);
break;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/vpp/run-interval.ts` around lines 67 - 93,
parseIntervalReadings currently accumulates consumption for any member resolved
via deviceToMember but input.members passed to fairSplit is filtered to only
active members, causing unpaired consumption and invariant failures in
fairSplit; fix by ensuring consumption keys match active members: either (a)
change parseIntervalReadings or a post-processing step to filter the returned
consumption object to only include addresses present in
members.filter(m=>m.isActive).map(m=>m.address), or (b) detect when a reading
maps to an inactive member and throw a clear error. Update the code paths around
parseIntervalReadings, the creation of input (members/consumption), and any
callers of fairSplit so that totalConsumption only includes active members (or
fails fast if inactive-member readings exist) before calling fairSplit and
buildConsumptionReadings.
|
🔗 Custom preview URL: https://pr-2190.preview-app.hypha.earth |
…ripts Adds the EnergyPPAv2 on-chain settlement stack for the community energy demo: - Contracts: EnergyPPAv2, EnergyPPAv2Factory, EnergyToken, RegularSpaceToken - VPP module: 3-pass fair-split allocator, contract reading builder, on-chain reader, interval orchestrator, types, and unit tests - Base mainnet scripts: factory deploy, all-in-one demo, VPP loop, and demo state JSON - Hardhat tests: functional + gas benchmarks for consumeEnergy - README-energy-demo.md describing the contracts, VPP, scripts, and settlement flow Made-with: Cursor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
♻️ Duplicate comments (9)
packages/storage-evm/test/EnergyPPAv2Functional.test.ts (3)
90-99:⚠️ Potential issue | 🔴 CriticalThe fixture still needs the new
gridOperatorinitializer arg.This proxy deployment still passes four initializer arguments, so the suite never gets past fixture setup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/test/EnergyPPAv2Functional.test.ts` around lines 90 - 99, The proxy initializer call for EnergyPPAv2 is missing the new gridOperator argument; update the deployProxy call that constructs EnergyPPAv2 (the array currently containing owner.address, energyToken.target, stablecoin.target, ethers.ZeroAddress) to include the gridOperator fixture value as the correct fifth initializer parameter (e.g., gridOperator.address or gridOperator.target instead of/after ethers.ZeroAddress) so the contract initialize signature matches the contract's initializer.
198-199:⚠️ Potential issue | 🔴 CriticalThese tests still use the removed import/export balance getters.
EnergyPPAv2has consolidated that accounting intogetGridBalance(). Keeping the old getters here leaves several cases uncompilable or permanently red.Also applies to: 501-501, 902-902, 1323-1324, 1444-1449
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/test/EnergyPPAv2Functional.test.ts` around lines 198 - 199, Tests call the removed getters getImportEnergyCreditBalance and getExportEnergyCreditBalance on EnergyPPAv2; replace those calls with the consolidated getGridBalance() and adjust usages accordingly (e.g., split or inspect the returned structure/fields from getGridBalance instead of expecting separate import/export values) in the referenced test locations (around the occurrences near the shown diff and at the other noted ranges). Locate calls to getImportEnergyCreditBalance and getExportEnergyCreditBalance in the test file and swap them to a single call to ppa.getGridBalance(), then update variable names, assertions and any destructuring to match the getGridBalance return shape. Ensure imports/types used in assertions match the new return type from getGridBalance.
1150-1202:⚠️ Potential issue | 🔴 CriticalThe factory test payload is still missing
gridOperator.
deployCommunity(...)now expects that field inCommunityParams, so this integration test is still constructing an invalid request object.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/test/EnergyPPAv2Functional.test.ts` around lines 1150 - 1202, The test payload passed to factory.deployCommunity is missing the required gridOperator field from CommunityParams; update the object in the test (the deployCommunity call) to include gridOperator: <appropriate address> (e.g., gridOperator.address or owner.address/community.address depending on the existing test fixture variable you have) so the constructed CommunityParams includes gridOperator alongside admin, stablecoin, communityAddress, etc.packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-vpp-loop.ts (2)
941-949:⚠️ Potential issue | 🔴 Critical
emergencyReset()still needs an explicit mainnet confirmation guard.
ENERGY_DEMO_COMMAND=resetwill still immediately send a destructive transaction against the configured proxy. This should be blocked on Base mainnet unless a separate confirmation flag is present.As per coding guidelines: "
packages/storage-evm/**: Verify: Deploy scripts are idempotent and environment-aware"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-vpp-loop.ts` around lines 941 - 949, The reset branch currently calls emergencyReset() immediately when command === 'reset', which allows ENERGY_DEMO_COMMAND=reset to send a destructive transaction on Base mainnet; add an explicit mainnet confirmation guard by detecting the chain/network (e.g., via provider.getNetwork() or chainId) and requiring an additional, unmistakable env flag or interactive confirmation (for example ENERGY_DEMO_CONFIRM_RESET=true or a prompt) before calling ppa.emergencyReset(); update the reset handling to abort with a clear message if running on the mainnet and the confirmation flag is not set, and reference the same symbols used here (command === 'reset' and ppa.emergencyReset()) so the guard is applied exactly where the transaction is triggered.
459-463: 🧹 Nitpick | 🔵 Trivial
anyis still leaking through the contract and error paths.The script is still using untyped
ppa/error values in the same places flagged earlier. Please replace these with the concrete Hardhat contract type andunknown+ narrowing so type errors don't get masked in the mainnet path.Based on learnings: "In hypha-dao/hypha-web, always flag usage of TypeScript
any... and do not approve/overlook it."Also applies to: 692-696, 824-827, 843-843, 966-968
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-vpp-loop.ts` around lines 459 - 463, The function printOnChainState (and the other spots noted) currently accepts and uses an untyped ppa value and caught errors with any; replace the loose types: change the ppa parameter type from any to the generated TypeChain contract interface for this PPA (e.g., EnergyPPA, PPAv2 or the specific contract name from your typechain) so calls on ppa are typed, and change caught error types from any to unknown and narrow them (use instanceof Error or check .message) before logging or accessing properties; update the other occurrences (around the commented locations) to the same pattern so no any leaks remain.packages/storage-evm/vpp/run-interval.ts (1)
67-80:⚠️ Potential issue | 🟠 MajorInactive-member consumption is still included in
consumptionbut excluded frominput.members.A reading routed through
deviceToMemberfor an inactive member will be added tototalConsumption, butfairSplit()never iterates that address becausemembersis active-only. That leaves unallocatable Wh and breaks the balance invariant.Also applies to: 111-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/vpp/run-interval.ts` around lines 67 - 80, The consumption totals include readings for members flagged inactive, but the FairSplitInput.members list only contains active addresses, causing unallocatable energy; fix by ensuring consumption/production keys match input.members: either update parseIntervalReadings to accept and use only active member addresses (from members.filter(m => m.isActive).map(m => m.address)) when computing totalConsumption/totalProduction, or after parseIntervalReadings prune any deviceToMember-aggregated entries in the returned consumption/production objects to keep only addresses present in the active-members array before constructing the FairSplitInput used by fairSplit().packages/storage-evm/contracts/EnergyPPAv2Factory.sol (1)
117-118:⚠️ Potential issue | 🔴 CriticalDo not hand
EnergyTokenownership to the admin.After this transfer,
p.admincan callsetAuthorized(...)and grant arbitrary mint/burn rights, which lets credits be created outsideEnergyPPAv2settlement. Ownership/minter governance needs to stay with the proxy or another tightly constrained controller.As per coding guidelines: "
packages/storage-evm/contracts/**: Check for ... access control vulnerabilities"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/contracts/EnergyPPAv2Factory.sol` around lines 117 - 118, The code currently calls energyToken.setAuthorized(proxy, true) then energyToken.transferOwnership(p.admin), which hands full token ownership to p.admin and allows them to call setAuthorized to grant mint/burn rights; instead keep ownership and minter control with the proxy (or another constrained controller) by removing or changing the transferOwnership(p.admin) call so ownership remains with proxy (or explicitly transferOwnership(proxy) / a governance-managed controller), and ensure setAuthorized is only granted for proxy; update references around energyToken.setAuthorized, energyToken.transferOwnership, proxy and p.admin in EnergyPPAv2Factory to preserve token ownership/minter governance with the proxy/controller rather than p.admin.packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts (2)
413-432:⚠️ Potential issue | 🟠 MajorThe household rebalance can still assign more solar than actually exists.
The fallback
hhSolar += ...adjustment is still there, so rounding can over-attribute solar across households beyondsolarUsed. That means the batch can charge for non-existent solar production instead of routing the residual to import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts` around lines 413 - 432, The hhSolar fallback can over-assign solar beyond actual solarUsed; instead of increasing hhSolar in the final adjustments, cap hhSolar to the remaining available solar and route any shortfall to hhImport (i.e., compute remainingSolar = solarUsed - hhSolar and set hhSolar = hhSolar + min(remainingSolar, demand - allocated) and hhImport += (demand - allocated) - min(remainingSolar, demand - allocated)); update the logic around solarShare, hhSolar, allocated, hhImport, importQty and the final total check so the residual is assigned to import rather than inflating hhSolar beyond solarUsed.
317-341: 🧹 Nitpick | 🔵 TrivialThe unresolved
anyusages are still in the deploy/demo path.The same
anycasts remain in the static-call error handling, log parsing, and contract helper signatures. Please replace them with concrete contract/log types andunknownerror narrowing before this script is used on a live network.Based on learnings: "In hypha-dao/hypha-web, always flag usage of TypeScript
any... and do not approve/overlook it."Also applies to: 492-492, 656-658, 777-779, 789-792
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts` around lines 317 - 341, Replace the remaining "any" usages in the deploy/demo flow: change the catch parameter from "e: any" to "e: unknown" and narrow it (e.g., check instanceof Error and fallback to String(e)) when logging the revert for factory.deployCommunity.staticCall; replace generic types for tx/receipt/log parsing by using concrete ethers types (e.g., the function deployCommunity return type from your factory contract, ethers.providers.TransactionResponse/TransactionReceipt for tx and receipt, and ethers.providers.Log for items in receipt.logs) and use factory.interface.parseLog with the proper parsed log type instead of "any"; update any helper signatures that reference deployCommunity or the parsed CommunityDeployed event to use the generated contract typings (the factory contract type and the CommunityDeployed event/log type) so all "any" casts are removed in static-call error handling, the receipt.logs mapping, and helper signatures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/storage-evm/contracts/EnergyPPAv2.sol`:
- Around line 633-667: Several admin setters lack emitted events making config
changes invisible to indexers; add and emit descriptive events for
setEnergyToken, setStablecoin, setCommunityAddress, setAggregatorAddress,
setCommunityFeeBps, setAggregatorFeeBps, and setPaymentRecipient (e.g.,
EnergyTokenSet(address), StablecoinSet(address), CommunityAddressSet(address),
AggregatorAddressSet(address), CommunityFeeBpsSet(uint16),
AggregatorFeeBpsSet(uint16), PaymentRecipientSet(address)); declare these events
in the contract and emit the appropriate event at the end of each setter with
the new value (and sender if useful) to ensure all state-changing admin calls
are recorded.
- Around line 365-396: _distributeSourceRevenue currently iterates
memberAddresses twice and repeatedly calls IERC20.balanceOf for each member
during every settlement, causing O(sources×members) gas growth; replace
on-the-fly enumeration with a scalable design: stop scanning memberAddresses in
_distributeSourceRevenue, and instead implement a pull-claim or
snapshot/registry pattern (e.g., a holders registry or periodic snapshot
mapping) that stores per-source holder shares (or balances at snapshot) so
_distributeSourceRevenue only iterates claimants or updates aggregated state;
update references to memberAddresses, balanceOf, lastHolder, remaining,
_adjustEnergyCreditBalance, and RevenueDistributed to consume the new
registry/snapshot and ensure last-recipient rounding logic still uses stored
totals rather than live balanceOf calls.
- Around line 283-289: The code treats any reading with deviceId ==
exportDeviceId as an export and can misclassify member devices (exportDeviceId
defaults to 0); update the logic and write paths so exportDeviceId is kept
disjoint from member devices: (1) in setExportDeviceId, validate the new
exportDeviceId is not already a member (reject/require), and prevent setting it
to the sentinel/default if that collides; (2) in addMember, reject adding a
member whose deviceId equals the current exportDeviceId; and (3) in the
export-detection logic (where r.deviceId == exportDeviceId and things like
IMPORT_SOURCE_ID are checked), ensure ownership/ membership is checked first or
that the exportDeviceId cannot equal any member id so misclassification cannot
occur; apply the same fixes to the other occurrences referenced (around the
comparisons at lines ~411-427 and ~642-644).
- Around line 700-709: In _setEnergyCreditBalance, when amount > 0 call
energyToken.mint(account, uint256(amount)) instead of energyToken.transfer(...)
to make intent explicit and save gas; update the positive branch in the
_setEnergyCreditBalance function to invoke mint on the energyToken contract
(energyToken.mint) and remove the transfer call so the contract uses direct
minting for credited balances.
In `@packages/storage-evm/README-energy-demo.md`:
- Around line 16-19: The README incorrectly states ConsumptionReading.quantity
is kWh; clarify that quantity is deployment-specific and may be in Wh or scaled
to kWh by vpp/build-readings.ts using quantityScale and that vpp/types.ts
describes the contract-internal unit; update the README entries (the
ConsumptionReading bullet and any examples) to say “quantity: numeric value in
the contract-internal unit (see vpp/types.ts); may be Wh or kWh depending on
deployment and/or quantityScale in vpp/build-readings.ts” and mention conversion
guidance to avoid callers assuming kWh.
In
`@packages/storage-evm/scripts/base-mainnet-contracts-scripts/deploy-energy-ppa-v2-factory.ts`:
- Around line 378-399: The code treats any non-empty ENERGY_PPAV2_FACTORY as
"reuse" even if it's not a valid address; update the logic that sets
existingFactory to validate the env value (use ethers.utils.isAddress or
similar) before skipping deployments — if the env value is present but invalid,
either throw an error or ignore it so deployFactory() will run normally;
specifically change the checks around existingFactory in the EnergyPPAv2/RST
blocks (symbols: ENERGY_PPAV2_FACTORY, existingFactory, deployFactory,
energyPPAv2Impl, rstImpl, resolveRSTImplementation) to validate the address and
fail-fast on bad input instead of setting impl vars to 'see factory'.
In
`@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts`:
- Around line 142-168: deriveActors currently falls back to
Wallet.createRandom() when ENERGY_TEST_MNEMONIC or ENERGY_ACTOR_PRIVATE_KEYS are
missing which risks creating uncontrolled addresses on mainnet; change
deriveActors to detect a mainnet deployment (or when process.env.NETWORK ===
'mainnet' / a provided isMainnet flag) and, in that case, refuse to proceed by
throwing a clear Error if neither ENERGY_TEST_MNEMONIC nor a sufficiently long
ENERGY_ACTOR_PRIVATE_KEYS list is provided instead of generating random
addresses; keep the existing random-generation behavior only for
non-mainnet/local runs, and update the error message to mention the required ENV
names (ENERGY_TEST_MNEMONIC, ENERGY_ACTOR_PRIVATE_KEYS) and the minimum keys
needed so callers know how to fix it (references: deriveActors,
ENERGY_TEST_MNEMONIC, ENERGY_ACTOR_PRIVATE_KEYS, Wallet.createRandom).
- Around line 733-747: The deploy branch currently always creates a new
community even when a same-chain state exists; update the logic around
deployFactoryIfNeeded, deployCommunity, saveState and the command handling to be
idempotent by detecting an existing state and skipping deploy unless an explicit
force flag is supplied (e.g. ENV var ENERGY_DEMO_FORCE=true or a CLI --force).
Concretely: before calling deployFactoryIfNeeded()/deployCommunity(), check the
loaded state variable and if it contains a deployed community for the current
chain/address, log and reuse it (return early) unless the force flag is truthy;
only when force is present proceed to call deployFactoryIfNeeded(),
deployCommunity(...), and saveState(state). Ensure the same guard applies to
both ENV-driven commands and 'deploy' command paths.
In
`@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-vpp-loop.ts`:
- Around line 173-219: buildSources and buildMembers reconstruct VPP inputs from
hardcoded demo assumptions which can diverge from on-chain state used by
consumeEnergy/fairSplit; replace these functions to query the on-chain reader
(the contract reader used elsewhere in the script) to fetch current sources,
ownership tokens and ownershipBps, member addresses, active flags and deviceId
mappings, and device assignments before calling fairSplit/consumeEnergy so the
off-chain allocation matches the live chain config; update buildSources to
populate sourceId, sourceType, basePricePerKwh, ownershipToken and ownershipBps
from reader methods (e.g., getSource/getOwnership or equivalent), and update
buildMembers to pull member active status and deviceIds from the reader (e.g.,
getMember/getDeviceMapping), ensuring you await and aggregate those reads and
handle absent data consistently.
- Around line 67-69: QUANTITY_SCALE = 1000 currently truncates Wh→kWh in
buildConsumptionReadings(...) causing under-submission; change the conversion
and allocation so fractional kWh aren't silently dropped: update
buildConsumptionReadings(...) (and callers like fairSplit()) to perform exact
division and then either use Math.round(wh / QUANTITY_SCALE) instead of flooring
or, better, compute integer kWh by floor and distribute the leftover Wh
remainders across allocations (e.g., add +1 kWh to the largest fractional
remainders) so the submitted batch totals equal the original Wh totals; keep the
QUANTITY_SCALE symbol but adjust conversion logic and ensure totals are
validated before submission.
In `@packages/storage-evm/test/EnergyPPAv2Gas.test.ts`:
- Around line 397-449: The test payload passed to factory.deployCommunity is
missing the required CommunityParams field gridOperator; update the object
literal in the deployCommunity call to include gridOperator (e.g., gridOperator:
owner.address or another appropriate signer/address) so the CommunityParams
shape matches the factory.deployCommunity signature and the factory call will
not fail during the deploy benchmark.
- Around line 1019-1024: Tests call the removed getters
getImportEnergyCreditBalance()/getExportEnergyCreditBalance(); replace those
calls with the consolidated getGridBalance() on the same ppa instance, then read
the import/export fields from the returned structure (e.g., result.importBalance
and/or result.exportBalance or whatever field names the new return type uses)
and update the assertions to compare against those values; do this for the
failing instances around the existing expect checks (including the two other
similar spots flagged) and leave verifyZeroSum() assertions unchanged.
- Around line 96-105: The proxy deployment is calling upgrades.deployProxy with
only four initializer args but EnergyPPAv2.initialize requires five (including
gridOperator); update the deployProxy calls (the ones that construct ppa in
EnergyPPAv2Gas.test and the other occurrences) to pass the missing fifth
argument—use the test fixture reference consistent with the other args (e.g.,
gridOperator.target or gridOperator.address depending on how gridOperator is
defined) as the fifth parameter so initializer: 'initialize' receives
owner.address, energyToken.target, stablecoin.target, ethers.ZeroAddress, and
gridOperator.
In `@packages/storage-evm/vpp/build-readings.ts`:
- Around line 37-70: The current loop eagerly does
memberDeviceMap.get(alloc.memberAddress) and throws if missing even when alloc
emits no readings; move the deviceId lookup and undefined check into the
branches that actually push readings so pure investors (no consumption) don't
abort the batch: perform memberDeviceMap.get(...) and throw only inside the
sourceAllocations branch when scaleQuantity(sa.usedWh, quantityScale) > 0n and
before pushing the reading, and likewise perform the lookup and check inside the
alloc.gridImportWh > 0 branch before creating the grid import reading; keep all
other logic (scaleQuantity, sourceMap lookup, pricePerKwh, IMPORT_SOURCE_ID,
readings.push) unchanged.
In `@packages/storage-evm/vpp/fair-split.test.ts`:
- Around line 515-548: The test initializes shared variables (result,
contractReadings) inside a prior it(...) which makes subsequent assertions
brittle; move the scenario setup into a dedicated before()/beforeEach() hook or
call a shared helper from each it so each test runs its own setup. Specifically,
create a setup that builds hhDemands, prodMap, readings via makeIntervalReadings
and constructs the FairSplitInput, then call runScenario to assign result and
contractReadings (references: result, contractReadings, runScenario,
FairSplitInput, makeIntervalReadings, ALL_MEMBERS) before the assertions in each
scenario test; apply the same change to the other scenario blocks mentioned
(lines ~604-630, ~662-692, ~723-749).
In `@packages/storage-evm/vpp/index.ts`:
- Around line 5-18: The barrel is missing exports for the trace types used by
the public API: add exports for TraceEvent and TraceFn so consumers of fairSplit
can import the callback types from the package root; update the export block
that currently re-exports types (IntervalReading, SourceInfo, etc.) to also
include TraceEvent and TraceFn (these names correspond to the types defined in
./types and referenced by fairSplit) so downstream callers don't have to import
from ./types directly.
In `@packages/storage-evm/vpp/types.ts`:
- Around line 34-55: The MemberInfo, OnChainConfig and VppConfig types use
number for on-chain device IDs which can overflow; change all device ID types to
bigint: update MemberInfo.deviceIds to bigint[], OnChainConfig.exportDeviceId to
bigint, OnChainConfig.deviceToMember to Map<bigint,string> (and any other
deviceId map keys), and VppConfig.productionDeviceToSource to
Map<bigint,string>; then update any call sites (e.g., on-chain-reader.ts and
build-readings.ts) to stop casting to Number and to use BigInt-safe
operations/lookup keys so the module consistently treats on-chain uint256 IDs as
bigint throughout.
---
Duplicate comments:
In `@packages/storage-evm/contracts/EnergyPPAv2Factory.sol`:
- Around line 117-118: The code currently calls energyToken.setAuthorized(proxy,
true) then energyToken.transferOwnership(p.admin), which hands full token
ownership to p.admin and allows them to call setAuthorized to grant mint/burn
rights; instead keep ownership and minter control with the proxy (or another
constrained controller) by removing or changing the transferOwnership(p.admin)
call so ownership remains with proxy (or explicitly transferOwnership(proxy) / a
governance-managed controller), and ensure setAuthorized is only granted for
proxy; update references around energyToken.setAuthorized,
energyToken.transferOwnership, proxy and p.admin in EnergyPPAv2Factory to
preserve token ownership/minter governance with the proxy/controller rather than
p.admin.
In
`@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts`:
- Around line 413-432: The hhSolar fallback can over-assign solar beyond actual
solarUsed; instead of increasing hhSolar in the final adjustments, cap hhSolar
to the remaining available solar and route any shortfall to hhImport (i.e.,
compute remainingSolar = solarUsed - hhSolar and set hhSolar = hhSolar +
min(remainingSolar, demand - allocated) and hhImport += (demand - allocated) -
min(remainingSolar, demand - allocated)); update the logic around solarShare,
hhSolar, allocated, hhImport, importQty and the final total check so the
residual is assigned to import rather than inflating hhSolar beyond solarUsed.
- Around line 317-341: Replace the remaining "any" usages in the deploy/demo
flow: change the catch parameter from "e: any" to "e: unknown" and narrow it
(e.g., check instanceof Error and fallback to String(e)) when logging the revert
for factory.deployCommunity.staticCall; replace generic types for tx/receipt/log
parsing by using concrete ethers types (e.g., the function deployCommunity
return type from your factory contract,
ethers.providers.TransactionResponse/TransactionReceipt for tx and receipt, and
ethers.providers.Log for items in receipt.logs) and use
factory.interface.parseLog with the proper parsed log type instead of "any";
update any helper signatures that reference deployCommunity or the parsed
CommunityDeployed event to use the generated contract typings (the factory
contract type and the CommunityDeployed event/log type) so all "any" casts are
removed in static-call error handling, the receipt.logs mapping, and helper
signatures.
In
`@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-vpp-loop.ts`:
- Around line 941-949: The reset branch currently calls emergencyReset()
immediately when command === 'reset', which allows ENERGY_DEMO_COMMAND=reset to
send a destructive transaction on Base mainnet; add an explicit mainnet
confirmation guard by detecting the chain/network (e.g., via
provider.getNetwork() or chainId) and requiring an additional, unmistakable env
flag or interactive confirmation (for example ENERGY_DEMO_CONFIRM_RESET=true or
a prompt) before calling ppa.emergencyReset(); update the reset handling to
abort with a clear message if running on the mainnet and the confirmation flag
is not set, and reference the same symbols used here (command === 'reset' and
ppa.emergencyReset()) so the guard is applied exactly where the transaction is
triggered.
- Around line 459-463: The function printOnChainState (and the other spots
noted) currently accepts and uses an untyped ppa value and caught errors with
any; replace the loose types: change the ppa parameter type from any to the
generated TypeChain contract interface for this PPA (e.g., EnergyPPA, PPAv2 or
the specific contract name from your typechain) so calls on ppa are typed, and
change caught error types from any to unknown and narrow them (use instanceof
Error or check .message) before logging or accessing properties; update the
other occurrences (around the commented locations) to the same pattern so no any
leaks remain.
In `@packages/storage-evm/test/EnergyPPAv2Functional.test.ts`:
- Around line 90-99: The proxy initializer call for EnergyPPAv2 is missing the
new gridOperator argument; update the deployProxy call that constructs
EnergyPPAv2 (the array currently containing owner.address, energyToken.target,
stablecoin.target, ethers.ZeroAddress) to include the gridOperator fixture value
as the correct fifth initializer parameter (e.g., gridOperator.address or
gridOperator.target instead of/after ethers.ZeroAddress) so the contract
initialize signature matches the contract's initializer.
- Around line 198-199: Tests call the removed getters
getImportEnergyCreditBalance and getExportEnergyCreditBalance on EnergyPPAv2;
replace those calls with the consolidated getGridBalance() and adjust usages
accordingly (e.g., split or inspect the returned structure/fields from
getGridBalance instead of expecting separate import/export values) in the
referenced test locations (around the occurrences near the shown diff and at the
other noted ranges). Locate calls to getImportEnergyCreditBalance and
getExportEnergyCreditBalance in the test file and swap them to a single call to
ppa.getGridBalance(), then update variable names, assertions and any
destructuring to match the getGridBalance return shape. Ensure imports/types
used in assertions match the new return type from getGridBalance.
- Around line 1150-1202: The test payload passed to factory.deployCommunity is
missing the required gridOperator field from CommunityParams; update the object
in the test (the deployCommunity call) to include gridOperator: <appropriate
address> (e.g., gridOperator.address or owner.address/community.address
depending on the existing test fixture variable you have) so the constructed
CommunityParams includes gridOperator alongside admin, stablecoin,
communityAddress, etc.
In `@packages/storage-evm/vpp/run-interval.ts`:
- Around line 67-80: The consumption totals include readings for members flagged
inactive, but the FairSplitInput.members list only contains active addresses,
causing unallocatable energy; fix by ensuring consumption/production keys match
input.members: either update parseIntervalReadings to accept and use only active
member addresses (from members.filter(m => m.isActive).map(m => m.address)) when
computing totalConsumption/totalProduction, or after parseIntervalReadings prune
any deviceToMember-aggregated entries in the returned consumption/production
objects to keep only addresses present in the active-members array before
constructing the FairSplitInput used by fairSplit().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2e57a8ca-4308-44be-a6d2-4cda8e9f3b8d
📒 Files selected for processing (17)
packages/storage-evm/README-energy-demo.mdpackages/storage-evm/contracts/EnergyPPAv2.solpackages/storage-evm/contracts/EnergyPPAv2Factory.solpackages/storage-evm/contracts/EnergyToken.solpackages/storage-evm/scripts/base-mainnet-contracts-scripts/deploy-energy-ppa-v2-factory.tspackages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-demo-state.jsonpackages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.tspackages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-vpp-loop.tspackages/storage-evm/test/EnergyPPAv2Functional.test.tspackages/storage-evm/test/EnergyPPAv2Gas.test.tspackages/storage-evm/vpp/build-readings.tspackages/storage-evm/vpp/fair-split.test.tspackages/storage-evm/vpp/fair-split.tspackages/storage-evm/vpp/index.tspackages/storage-evm/vpp/on-chain-reader.tspackages/storage-evm/vpp/run-interval.tspackages/storage-evm/vpp/types.ts
Comment on lines
+283
to
+289
| if (r.deviceId == exportDeviceId) { | ||
| require(r.sourceId != IMPORT_SOURCE_ID, 'Cannot export import'); | ||
| uint256 revenue = r.quantity * r.pricePerKwh; | ||
| _addSourceRevenue(srcRevenues, r.sourceId, revenue); | ||
| gridBalance += int256(revenue); | ||
| emit EnergyExported(r.quantity, revenue, r.sourceId); | ||
| continue; |
Contributor
There was a problem hiding this comment.
Keep exportDeviceId disjoint from member devices.
Line 283 treats any reading whose deviceId matches exportDeviceId as an export before checking ownership. Because exportDeviceId defaults to 0, and neither addMember nor setExportDeviceId reject collisions, a real member device can be silently misclassified as grid export and skip consumer charging entirely. Reserve a sentinel/configured state and reject overlap in both write paths.
Also applies to: 411-427, 642-644
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/contracts/EnergyPPAv2.sol` around lines 283 - 289, The
code treats any reading with deviceId == exportDeviceId as an export and can
misclassify member devices (exportDeviceId defaults to 0); update the logic and
write paths so exportDeviceId is kept disjoint from member devices: (1) in
setExportDeviceId, validate the new exportDeviceId is not already a member
(reject/require), and prevent setting it to the sentinel/default if that
collides; (2) in addMember, reject adding a member whose deviceId equals the
current exportDeviceId; and (3) in the export-detection logic (where r.deviceId
== exportDeviceId and things like IMPORT_SOURCE_ID are checked), ensure
ownership/ membership is checked first or that the exportDeviceId cannot equal
any member id so misclassification cannot occur; apply the same fixes to the
other occurrences referenced (around the comparisons at lines ~411-427 and
~642-644).
Comment on lines
+365
to
+396
| address tokenAddr = sources[sourceId].ownershipToken; | ||
| IERC20 token = IERC20(tokenAddr); | ||
| uint256 totalSupply = token.totalSupply(); | ||
| if (totalSupply == 0) return; | ||
|
|
||
| address lastHolder = address(0); | ||
| for (uint256 i = 0; i < memberAddresses.length; i++) { | ||
| if (token.balanceOf(memberAddresses[i]) > 0) { | ||
| lastHolder = memberAddresses[i]; | ||
| } | ||
| } | ||
| if (lastHolder == address(0)) return; | ||
|
|
||
| uint256 distributed = 0; | ||
| for (uint256 i = 0; i < memberAddresses.length; i++) { | ||
| address addr = memberAddresses[i]; | ||
| uint256 tokenBal = token.balanceOf(addr); | ||
| if (tokenBal == 0) continue; | ||
|
|
||
| uint256 share; | ||
| if (addr == lastHolder) { | ||
| share = remaining - distributed; | ||
| } else { | ||
| share = (remaining * tokenBal) / totalSupply; | ||
| } | ||
| distributed += share; | ||
|
|
||
| if (share > 0) { | ||
| _adjustEnergyCreditBalance(addr, int256(share)); | ||
| emit RevenueDistributed(addr, share, sourceId, totalRevenue); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
This settlement path will hit a gas wall as the community grows.
_distributeSourceRevenue walks memberAddresses twice and does repeated balanceOf calls for every source with revenue. Since consumeEnergy triggers this on the interval hot path, the write cost grows with sources × members and will eventually make settlement uncallable on mainnet. This needs a pull-claim, snapshot, or holder-registry design instead of enumerating every holder during each settlement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/contracts/EnergyPPAv2.sol` around lines 365 - 396,
_distributeSourceRevenue currently iterates memberAddresses twice and repeatedly
calls IERC20.balanceOf for each member during every settlement, causing
O(sources×members) gas growth; replace on-the-fly enumeration with a scalable
design: stop scanning memberAddresses in _distributeSourceRevenue, and instead
implement a pull-claim or snapshot/registry pattern (e.g., a holders registry or
periodic snapshot mapping) that stores per-source holder shares (or balances at
snapshot) so _distributeSourceRevenue only iterates claimants or updates
aggregated state; update references to memberAddresses, balanceOf, lastHolder,
remaining, _adjustEnergyCreditBalance, and RevenueDistributed to consume the new
registry/snapshot and ensure last-recipient rounding logic still uses stored
totals rather than live balanceOf calls.
Comment on lines
+633
to
+667
| function setEnergyToken(address tokenAddress) external onlyWhitelist { | ||
| require(tokenAddress != address(0), 'Invalid address'); | ||
| energyToken = EnergyToken(tokenAddress); | ||
| } | ||
|
|
||
| function setStablecoin(address tokenAddress) external onlyWhitelist { | ||
| stablecoinAddress = tokenAddress; | ||
| } | ||
|
|
||
| function setExportDeviceId(uint256 deviceId) external onlyWhitelist { | ||
| exportDeviceId = deviceId; | ||
| emit ExportDeviceIdSet(deviceId); | ||
| } | ||
|
|
||
| function setCommunityAddress(address addr) external onlyOwner { | ||
| communityAddress = addr; | ||
| } | ||
|
|
||
| function setAggregatorAddress(address addr) external onlyOwner { | ||
| aggregatorAddress = addr; | ||
| } | ||
|
|
||
| function setCommunityFeeBps(uint16 bps) external onlyOwner { | ||
| require(bps + aggregatorFeeBps <= 10000, 'Total fees exceed 100%'); | ||
| communityFeeBps = bps; | ||
| } | ||
|
|
||
| function setAggregatorFeeBps(uint16 bps) external onlyOwner { | ||
| require(communityFeeBps + bps <= 10000, 'Total fees exceed 100%'); | ||
| aggregatorFeeBps = bps; | ||
| } | ||
|
|
||
| function setPaymentRecipient(address recipient) external onlyOwner { | ||
| paymentRecipient = recipient; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Emit events for the remaining config mutations.
setEnergyToken, setStablecoin, setCommunityAddress, setAggregatorAddress, setCommunityFeeBps, setAggregatorFeeBps, and setPaymentRecipient all mutate critical configuration without an event. That leaves indexers and incident response blind to admin changes. As per coding guidelines, "Events emitted for all state-changing operations".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/contracts/EnergyPPAv2.sol` around lines 633 - 667,
Several admin setters lack emitted events making config changes invisible to
indexers; add and emit descriptive events for setEnergyToken, setStablecoin,
setCommunityAddress, setAggregatorAddress, setCommunityFeeBps,
setAggregatorFeeBps, and setPaymentRecipient (e.g., EnergyTokenSet(address),
StablecoinSet(address), CommunityAddressSet(address),
AggregatorAddressSet(address), CommunityFeeBpsSet(uint16),
AggregatorFeeBpsSet(uint16), PaymentRecipientSet(address)); declare these events
in the contract and emit the appropriate event at the end of each setter with
the new value (and sender if useful) to ensure all state-changing admin calls
are recorded.
Comment on lines
+700
to
+709
| function _setEnergyCreditBalance(address account, int256 amount) internal { | ||
| uint256 current = energyToken.balanceOf(account); | ||
| if (current > 0) energyToken.burn(account, current); | ||
| energyCreditBalances[account] = 0; | ||
|
|
||
| if (amount > 0) { | ||
| energyToken.transfer(account, uint256(amount)); | ||
| } else if (amount < 0) { | ||
| energyCreditBalances[account] = amount; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== EnergyToken API ==="
sed -n '1,220p' packages/storage-evm/contracts/EnergyToken.sol
echo
echo "=== EnergyToken writes inside EnergyPPAv2 ==="
rg -n --type=sol 'energyToken\.(mint|burn|transfer)\(' packages/storage-evm/contracts/EnergyPPAv2.sol
echo
echo "=== Any place EnergyPPAv2 receives EnergyToken inventory ==="
rg -n --type=sol 'energyToken\.(mint|transferFrom)\(address\(this\)|energyToken\.mint\(address\(this\)|energyToken\.transferFrom\(.*address\(this\)' packages/storage-evm/contractsRepository: hypha-dao/hypha-web
Length of output: 3307
🏁 Script executed:
# Find energyToken calls in EnergyPPAv2.sol
rg -n 'energyToken\.(mint|burn|transfer)' packages/storage-evm/contracts/EnergyPPAv2.sol
# Check for authorization of EnergyPPAv2
rg -n 'setAuthorized' packages/storage-evm/contracts/
# Show the complete _setEnergyCreditBalance function
rg -A 15 'function _setEnergyCreditBalance' packages/storage-evm/contracts/EnergyPPAv2.solRepository: hypha-dao/hypha-web
Length of output: 1065
🏁 Script executed:
# Check EnergyPPAv2Factory to confirm proxy is EnergyPPAv2
rg -B 5 -A 5 'energyToken.setAuthorized.*proxy' packages/storage-evm/contracts/EnergyPPAv2Factory.solRepository: hypha-dao/hypha-web
Length of output: 428
Use mint() directly instead of transfer() for clarity and gas efficiency.
In _setEnergyCreditBalance, the positive balance case calls energyToken.transfer(account, uint256(amount)). While this will not revert (the transfer function's auto-mint logic for authorized senders covers it), using explicit mint() is clearer and more efficient: it avoids the unnecessary balance checks and auto-mint conditional logic in the transfer function.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/contracts/EnergyPPAv2.sol` around lines 700 - 709, In
_setEnergyCreditBalance, when amount > 0 call energyToken.mint(account,
uint256(amount)) instead of energyToken.transfer(...) to make intent explicit
and save gas; update the positive branch in the _setEnergyCreditBalance function
to invoke mint on the energyToken contract (energyToken.mint) and remove the
transfer call so the contract uses direct minting for credited balances.
Comment on lines
+16
to
+19
| - **ConsumptionReading**: `{ deviceId, quantity (kWh), pricePerKwh (ct/kWh), sourceId }`. The charge = quantity × price (euro cents). | ||
| - **gridBalance**: Positive = grid owes community (net exporter). Negative = community owes grid (net importer). | ||
| - **Zero-sum invariant**: Sum of all member credits + community fee credit + aggregator fee credit − gridBalance + settledBalance = 0. Always. | ||
| - **1 credit = 1 euro cent**. Stablecoin conversion: 1 credit × 10,000 = stablecoin base units (for 6-decimal tokens like EURC). |
Contributor
There was a problem hiding this comment.
Clarify that quantity is deployment-specific, not always kWh.
This bullet says ConsumptionReading.quantity is kWh, but vpp/build-readings.ts can emit either raw Wh or scaled kWh via quantityScale, and vpp/types.ts documents the field as a contract-internal unit. Leaving it as always-kWh makes the integration easy to call at the wrong scale.
📝 Proposed wording
-- **ConsumptionReading**: `{ deviceId, quantity (kWh), pricePerKwh (ct/kWh), sourceId }`. The charge = quantity × price (euro cents).
+- **ConsumptionReading**: `{ deviceId, quantity, pricePerKwh, sourceId }`, where `quantity` is the contract's configured energy unit (for example kWh when `quantityScale=1000`, otherwise Wh). The charge = quantity × price in internal credit units.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/README-energy-demo.md` around lines 16 - 19, The README
incorrectly states ConsumptionReading.quantity is kWh; clarify that quantity is
deployment-specific and may be in Wh or scaled to kWh by vpp/build-readings.ts
using quantityScale and that vpp/types.ts describes the contract-internal unit;
update the README entries (the ConsumptionReading bullet and any examples) to
say “quantity: numeric value in the contract-internal unit (see vpp/types.ts);
may be Wh or kWh depending on deployment and/or quantityScale in
vpp/build-readings.ts” and mention conversion guidance to avoid callers assuming
kWh.
Comment on lines
+1019
to
+1024
| // Import balance absorbs the charge | ||
| const importBal = await ppa.getImportEnergyCreditBalance(); | ||
| expect(importBal).to.equal(1250n); | ||
|
|
||
| const [ok] = await ppa.verifyZeroSum(); | ||
| expect(ok).to.be.true; |
Contributor
There was a problem hiding this comment.
Use the consolidated grid ledger getter here.
These tests still call getImportEnergyCreditBalance() / getExportEnergyCreditBalance(), but the contract surface has moved to a single getGridBalance(). As written, these cases won't compile or will fail at runtime.
Also applies to: 1065-1066, 1379-1380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/test/EnergyPPAv2Gas.test.ts` around lines 1019 - 1024,
Tests call the removed getters
getImportEnergyCreditBalance()/getExportEnergyCreditBalance(); replace those
calls with the consolidated getGridBalance() on the same ppa instance, then read
the import/export fields from the returned structure (e.g., result.importBalance
and/or result.exportBalance or whatever field names the new return type uses)
and update the assertions to compare against those values; do this for the
failing instances around the existing expect checks (including the two other
similar spots flagged) and leave verifyZeroSum() assertions unchanged.
Comment on lines
+37
to
+70
| for (const alloc of result.allocations) { | ||
| const deviceId = memberDeviceMap.get(alloc.memberAddress); | ||
| if (deviceId === undefined) { | ||
| throw new Error(`No device ID found for member ${alloc.memberAddress}`); | ||
| } | ||
|
|
||
| for (const sa of alloc.sourceAllocations) { | ||
| const source = sourceMap.get(sa.sourceId); | ||
| if (!source) { | ||
| throw new Error(`Unknown source ${sa.sourceId}`); | ||
| } | ||
|
|
||
| const quantity = scaleQuantity(sa.usedWh, quantityScale); | ||
| if (quantity > 0n) { | ||
| readings.push({ | ||
| deviceId: BigInt(deviceId), | ||
| quantity, | ||
| pricePerKwh: source.basePricePerKwh, | ||
| sourceId: sa.sourceId, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| if (alloc.gridImportWh > 0) { | ||
| const quantity = scaleQuantity(alloc.gridImportWh, quantityScale); | ||
| if (quantity > 0n) { | ||
| readings.push({ | ||
| deviceId: BigInt(deviceId), | ||
| quantity, | ||
| pricePerKwh: gridImportPrice, | ||
| sourceId: IMPORT_SOURCE_ID, | ||
| }); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Don't require a device ID before you know a reading will be emitted.
FairSplitInput.members explicitly includes pure investors with 0 consumption, and this PR's demo deploy script creates those investors with deviceIds: []. Throwing at the top of the allocation loop means a zero-usage allocation for those members aborts the whole batch even though nothing needs to be submitted for them.
♻️ Proposed lazy lookup
for (const alloc of result.allocations) {
- const deviceId = memberDeviceMap.get(alloc.memberAddress);
- if (deviceId === undefined) {
- throw new Error(`No device ID found for member ${alloc.memberAddress}`);
- }
+ let deviceId: number | undefined;
+ const getDeviceId = (): number => {
+ if (deviceId === undefined) {
+ deviceId = memberDeviceMap.get(alloc.memberAddress);
+ if (deviceId === undefined) {
+ throw new Error(`No device ID found for member ${alloc.memberAddress}`);
+ }
+ }
+ return deviceId;
+ };
for (const sa of alloc.sourceAllocations) {
const source = sourceMap.get(sa.sourceId);
if (!source) {
throw new Error(`Unknown source ${sa.sourceId}`);
@@
if (quantity > 0n) {
readings.push({
- deviceId: BigInt(deviceId),
+ deviceId: BigInt(getDeviceId()),
quantity,
pricePerKwh: source.basePricePerKwh,
sourceId: sa.sourceId,
});
}
@@
if (alloc.gridImportWh > 0) {
const quantity = scaleQuantity(alloc.gridImportWh, quantityScale);
if (quantity > 0n) {
readings.push({
- deviceId: BigInt(deviceId),
+ deviceId: BigInt(getDeviceId()),
quantity,
pricePerKwh: gridImportPrice,
sourceId: IMPORT_SOURCE_ID,
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/vpp/build-readings.ts` around lines 37 - 70, The current
loop eagerly does memberDeviceMap.get(alloc.memberAddress) and throws if missing
even when alloc emits no readings; move the deviceId lookup and undefined check
into the branches that actually push readings so pure investors (no consumption)
don't abort the batch: perform memberDeviceMap.get(...) and throw only inside
the sourceAllocations branch when scaleQuantity(sa.usedWh, quantityScale) > 0n
and before pushing the reading, and likewise perform the lookup and check inside
the alloc.gridImportWh > 0 branch before creating the grid import reading; keep
all other logic (scaleQuantity, sourceMap lookup, pricePerKwh, IMPORT_SOURCE_ID,
readings.push) unchanged.
Comment on lines
+515
to
+548
| let result: FairSplitResult; | ||
| let contractReadings: ConsumptionReading[]; | ||
|
|
||
| it('runs the full pipeline', () => { | ||
| const hhDemands = [20000, 18000, 15000, 12000, 15000]; | ||
| const prodMap = new Map([ | ||
| [SOLAR, 60000], | ||
| [BAT1, 10000], | ||
| [BAT2, 8000], | ||
| ]); | ||
| const readings = makeIntervalReadings(hhDemands, prodMap); | ||
|
|
||
| const input: FairSplitInput = { | ||
| members: ALL_MEMBERS, | ||
| consumption: new Map([ | ||
| [HH1, 20000], | ||
| [HH2, 18000], | ||
| [HH3, 15000], | ||
| [HH4, 12000], | ||
| [HH5, 15000], | ||
| ]), | ||
| production: prodMap, | ||
| sources: communitySources(), | ||
| gridImportPrice: PRICE_IMPORT, | ||
| gridExportPrice: PRICE_EXPORT, | ||
| }; | ||
|
|
||
| console.log('\n' + '═'.repeat(72)); | ||
| console.log(' SCENARIO 1 — Import (battery fallback + multi-iteration)'); | ||
| console.log(' Solar 60k, Bat2 8k, Bat1 10k = 78k Wh | Demand 80k Wh'); | ||
| console.log('═'.repeat(72)); | ||
|
|
||
| ({ result, contractReadings } = runScenario(input, readings)); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Avoid making the assertions depend on a separate setup test.
result and contractReadings are initialized in a preceding it(...), so a filtered run, a retry, or one setup failure turns the rest of the scenario into undefined-state noise instead of a real assertion failure. Move each scenario setup into before() or a shared helper invoked by the assertions.
Also applies to: 604-630, 662-692, 723-749
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/vpp/fair-split.test.ts` around lines 515 - 548, The test
initializes shared variables (result, contractReadings) inside a prior it(...)
which makes subsequent assertions brittle; move the scenario setup into a
dedicated before()/beforeEach() hook or call a shared helper from each it so
each test runs its own setup. Specifically, create a setup that builds
hhDemands, prodMap, readings via makeIntervalReadings and constructs the
FairSplitInput, then call runScenario to assign result and contractReadings
(references: result, contractReadings, runScenario, FairSplitInput,
makeIntervalReadings, ALL_MEMBERS) before the assertions in each scenario test;
apply the same change to the other scenario blocks mentioned (lines ~604-630,
~662-692, ~723-749).
Comment on lines
+5
to
+18
| export type { | ||
| IntervalReading, | ||
| SourceInfo, | ||
| MemberInfo, | ||
| OnChainConfig, | ||
| VppConfig, | ||
| FairSplitInput, | ||
| FairSplitResult, | ||
| MemberAllocation, | ||
| SourceAllocation, | ||
| ExportAllocation, | ||
| ConsumptionReading, | ||
| } from './types'; | ||
| export type { GridPrices, RunIntervalOptions } from './run-interval'; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Re-export the trace types from the barrel.
fairSplit exposes a trace callback, but the public entry point omits TraceEvent and TraceFn, so consumers have to reach into ./types to type that API.
♻️ Proposed export completion
export type {
IntervalReading,
SourceInfo,
MemberInfo,
OnChainConfig,
VppConfig,
FairSplitInput,
FairSplitResult,
MemberAllocation,
SourceAllocation,
ExportAllocation,
+ TraceEvent,
+ TraceFn,
ConsumptionReading,
} from './types';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export type { | |
| IntervalReading, | |
| SourceInfo, | |
| MemberInfo, | |
| OnChainConfig, | |
| VppConfig, | |
| FairSplitInput, | |
| FairSplitResult, | |
| MemberAllocation, | |
| SourceAllocation, | |
| ExportAllocation, | |
| ConsumptionReading, | |
| } from './types'; | |
| export type { GridPrices, RunIntervalOptions } from './run-interval'; | |
| export type { | |
| IntervalReading, | |
| SourceInfo, | |
| MemberInfo, | |
| OnChainConfig, | |
| VppConfig, | |
| FairSplitInput, | |
| FairSplitResult, | |
| MemberAllocation, | |
| SourceAllocation, | |
| ExportAllocation, | |
| TraceEvent, | |
| TraceFn, | |
| ConsumptionReading, | |
| } from './types'; | |
| export type { GridPrices, RunIntervalOptions } from './run-interval'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/vpp/index.ts` around lines 5 - 18, The barrel is missing
exports for the trace types used by the public API: add exports for TraceEvent
and TraceFn so consumers of fairSplit can import the callback types from the
package root; update the export block that currently re-exports types
(IntervalReading, SourceInfo, etc.) to also include TraceEvent and TraceFn
(these names correspond to the types defined in ./types and referenced by
fairSplit) so downstream callers don't have to import from ./types directly.
Comment on lines
+34
to
+55
| export interface MemberInfo { | ||
| address: string; | ||
| deviceIds: number[]; | ||
| isActive: boolean; | ||
| } | ||
|
|
||
| /** Full on-chain state snapshot needed by the VPP. */ | ||
| export interface OnChainConfig { | ||
| contractAddress: string; | ||
| sources: SourceInfo[]; | ||
| members: MemberInfo[]; | ||
| exportDeviceId: number; | ||
| communityFeeBps: number; | ||
| aggregatorFeeBps: number; | ||
| deviceToMember: Map<number, string>; // deviceId -> memberAddress | ||
| } | ||
|
|
||
| /** VPP-specific configuration not stored on-chain. */ | ||
| export interface VppConfig { | ||
| /** Maps production meter IDs to on-chain source IDs. */ | ||
| productionDeviceToSource: Map<number, string>; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 'deviceIds: number\[\]|exportDeviceId: number|deviceToMember: Map<number, string>|productionDeviceToSource: Map<number, string>|Number\(exportDeviceIdBn\)|map\(Number\)|BigInt\(deviceId\)' packages/storage-evm/vppRepository: hypha-dao/hypha-web
Length of output: 3059
🏁 Script executed:
#!/bin/bash
# Search for broader device ID patterns and usages
echo "=== Checking for other potential device ID usages ==="
rg -n 'deviceId|deviceIds' packages/storage-evm/vpp --type ts --type tsx | head -40
echo ""
echo "=== Checking build-readings.ts function signature ==="
rg -B5 'function buildReadings' packages/storage-evm/vpp/build-readings.ts | head -20Repository: hypha-dao/hypha-web
Length of output: 218
🏁 Script executed:
#!/bin/bash
# Simpler search for device ID usages
echo "=== Device ID usages in vpp directory ==="
rg 'deviceId' packages/storage-evm/vpp -n | head -50
echo ""
echo "=== Check build-readings.ts line count and view parameter ==="
wc -l packages/storage-evm/vpp/build-readings.ts
echo ""
echo "=== Extract buildReadings function signature ==="
sed -n '20,35p' packages/storage-evm/vpp/build-readings.tsRepository: hypha-dao/hypha-web
Length of output: 3592
Keep on-chain device IDs as bigint throughout the VPP module.
These fields model contract uint256 identifiers, but number cannot safely represent all on-chain values. The current shape forces lossy Number(...) downcasts in on-chain-reader.ts and repeated BigInt(...) upcasts in build-readings.ts, which can misroute readings once an ID exceeds Number.MAX_SAFE_INTEGER.
♻️ Proposed type fixes
types.ts:
export interface MemberInfo {
address: string;
- deviceIds: number[];
+ deviceIds: bigint[];
isActive: boolean;
}
/** Full on-chain state snapshot needed by the VPP. */
export interface OnChainConfig {
contractAddress: string;
sources: SourceInfo[];
members: MemberInfo[];
- exportDeviceId: number;
+ exportDeviceId: bigint;
communityFeeBps: number;
aggregatorFeeBps: number;
- deviceToMember: Map<number, string>; // deviceId -> memberAddress
+ deviceToMember: Map<bigint, string>; // deviceId -> memberAddress
}
/** VPP-specific configuration not stored on-chain. */
export interface VppConfig {
/** Maps production meter IDs to on-chain source IDs. */
- productionDeviceToSource: Map<number, string>;
+ productionDeviceToSource: Map<bigint, string>;
}build-readings.ts:
export function buildConsumptionReadings(
result: FairSplitResult,
sources: SourceInfo[],
members: MemberInfo[],
- exportDeviceId: number,
+ exportDeviceId: bigint,
gridImportPrice: bigint,
gridExportPrice: bigint,
quantityScale: number = 1,
): ConsumptionReading[] {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export interface MemberInfo { | |
| address: string; | |
| deviceIds: number[]; | |
| isActive: boolean; | |
| } | |
| /** Full on-chain state snapshot needed by the VPP. */ | |
| export interface OnChainConfig { | |
| contractAddress: string; | |
| sources: SourceInfo[]; | |
| members: MemberInfo[]; | |
| exportDeviceId: number; | |
| communityFeeBps: number; | |
| aggregatorFeeBps: number; | |
| deviceToMember: Map<number, string>; // deviceId -> memberAddress | |
| } | |
| /** VPP-specific configuration not stored on-chain. */ | |
| export interface VppConfig { | |
| /** Maps production meter IDs to on-chain source IDs. */ | |
| productionDeviceToSource: Map<number, string>; | |
| } | |
| export interface MemberInfo { | |
| address: string; | |
| deviceIds: bigint[]; | |
| isActive: boolean; | |
| } | |
| /** Full on-chain state snapshot needed by the VPP. */ | |
| export interface OnChainConfig { | |
| contractAddress: string; | |
| sources: SourceInfo[]; | |
| members: MemberInfo[]; | |
| exportDeviceId: bigint; | |
| communityFeeBps: number; | |
| aggregatorFeeBps: number; | |
| deviceToMember: Map<bigint, string>; // deviceId -> memberAddress | |
| } | |
| /** VPP-specific configuration not stored on-chain. */ | |
| export interface VppConfig { | |
| /** Maps production meter IDs to on-chain source IDs. */ | |
| productionDeviceToSource: Map<bigint, string>; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-evm/vpp/types.ts` around lines 34 - 55, The MemberInfo,
OnChainConfig and VppConfig types use number for on-chain device IDs which can
overflow; change all device ID types to bigint: update MemberInfo.deviceIds to
bigint[], OnChainConfig.exportDeviceId to bigint, OnChainConfig.deviceToMember
to Map<bigint,string> (and any other deviceId map keys), and
VppConfig.productionDeviceToSource to Map<bigint,string>; then update any call
sites (e.g., on-chain-reader.ts and build-readings.ts) to stop casting to Number
and to use BigInt-safe operations/lookup keys so the module consistently treats
on-chain uint256 IDs as bigint throughout.
Updated the energy route logic to ensure ownership basis points are safely handled, defaulting to 0 when undefined. Refactored the community activation mapping to return null if the upsert operation fails. Enhanced source result retrieval by using Promise.all for improved performance and clarity in handling source data.
 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 28
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/epics/src/proposals/components/proposal-detail.tsx (1)
1106-1427:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStrip the energy marker before rendering the body.
You decode the energy payload here, but
MarkdownSuspensestill receives the originalcontent, so users will see the encoded marker in the proposal body in addition toProposalEnergyProposalData.Please apply the same “parse + strip” pattern used for the investment/exchange flows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/proposals/components/proposal-detail.tsx` around lines 1106 - 1427, The proposal body currently displays the raw energy marker because MarkdownSuspense is passed content unchanged; update the render to strip the energy marker before passing to MarkdownSuspense using the same pattern as Investment/Exchange: when label is one of 'Energy Sharing' | 'Register Energy Source' | 'Add Energy Member' detect/parse via parseEnergyProposalMarker (or reuse energyMarkerData) and call the corresponding strip function (e.g., stripEnergyProposalMarker) to produce strippedContent, then pass strippedContent to MarkdownSuspense (fall back to content when parsing fails); ensure you reference energyMarkerData, parseEnergyProposalMarker, and the new/expected stripEnergyProposalMarker in the change.
♻️ Duplicate comments (4)
packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts (4)
320-341: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winRemove remaining explicit
anyfrom error/log/contract typing paths.Line 320, Line 331, Line 341, Line 492, and Line 656 still use
any, which weakens type safety exactly where failures/events are handled.Typed alternative
+import type { Log, LogDescription } from 'ethers'; @@ - } catch (e: any) { - console.error(` staticCall REVERTED: ${e.reason ?? e.message}`); + } catch (e: unknown) { + const err = e as { reason?: string; message?: string }; + console.error(` staticCall REVERTED: ${err.reason ?? err.message}`); throw e; } @@ - const event = receipt.logs - .map((log: any) => { + const event = receipt.logs + .map((log: Log): LogDescription | null => { try { return factory.interface.parseLog({ - topics: log.topics as string[], + topics: [...log.topics], data: log.data, }); } catch { return null; } }) - .find((e: any) => e?.name === 'CommunityDeployed'); + .find((parsed): parsed is LogDescription => parsed?.name === 'CommunityDeployed'); @@ -async function printFullState( - ppa: any, +interface PpaReadApi { + getSourceOwnershipBps(sourceId: string, member: string): Promise<bigint>; + getEnergyCreditBalance(member: string): Promise<bigint>; + getGridBalance(): Promise<bigint>; + getSettledBalance(): Promise<bigint>; + verifyZeroSum(): Promise<[boolean, bigint]>; +} + +async function printFullState( + ppa: PpaReadApi, state: DemoState, heading: string, ): Promise<void> { @@ - } catch (e: any) { - console.error(`\n staticCall reverted: ${e.reason ?? e.message}`); + } catch (e: unknown) { + const err = e as { reason?: string; message?: string }; + console.error(`\n staticCall reverted: ${err.reason ?? err.message}`); throw e; }#!/bin/bash set -euo pipefail fd -i 'energy-ppav2-mainnet-demo.ts' packages/storage-evm/scripts --exec rg -n '\b:\s*any\b|catch\s*\(\s*e\s*:\s*any\s*\)'Based on learnings: "always flag usage of TypeScript
any... and suggest properly typed alternatives ... consistently across the repo’s TypeScript/TSX files."Also applies to: 492-492, 656-658
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts` around lines 320 - 341, Replace the remaining explicit any usages by using proper types: change catch (e: any) to catch (e: unknown) and normalize the error into a message/reason (e.g., if (e instanceof Error) use e.message else String(e)); type receipt.logs as ethers.providers.Log[] and import that type so receipt is checked as TransactionReceipt; when parsing logs, type the parsed result as ethers.utils.LogDescription (the return of factory.interface.parseLog) and avoid any by mapping receipt.logs as Log[] and filtering for (ld?: LogDescription) => ld?.name === 'CommunityDeployed'; update references to factory.interface.parseLog, receipt, logs, and event to use these concrete types instead of any.
413-432:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHousehold rebalancing can oversubscribe global solar/import totals.
Line 424-432 fills deficits per household without decrementing shared remaining budgets, so summed
hhImport/hhSolarcan exceedimportQty/solarUsedafter rounding. That can produce readings inconsistent with planned source totals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts` around lines 413 - 432, The household rebalancing can oversubscribe shared source budgets because after computing per-household shares (solarShare, bat1Share, bat2Share) the code adjusts hhSolar/hhImport without updating remaining global budgets (solarUsed/importQty), allowing summed hhSolar/hhImport to exceed solarUsed/importQty; fix by tracking and decrementing remainingSolar and remainingImport before assigning hhImport or topping hhSolar (use remainingSolar = solarUsed - hhSolar and remainingImport = importQty - allocatedImport) and clamp hhImport and hhSolar to those remaining amounts (ensuring hhImport = min(demand - allocated, remainingImport) and any leftover demand after import is taken from remainingSolar up to remainingSolar) and finally recompute total and adjust hhSolar by any tiny residual only if remainingSolar permits so that summed hh* never exceed original solarUsed/importQty.
733-747:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
ENERGY_DEMO_COMMAND=deployis still non-idempotent with existing state.With a same-chain state file present, this branch still deploys a new community and overwrites state. Add an explicit force gate and reuse existing state by default.
One minimal guard
+ const forceDeploy = getEnv('ENERGY_DEMO_FORCE_DEPLOY') === '1'; + if (command === 'deploy' && state && !forceDeploy) { + printSetup(state); + console.log('\nExisting state found. Set ENERGY_DEMO_FORCE_DEPLOY=1 to redeploy.'); + return; + } + if (command === 'deploy' || (!state && !skipAutoDeploy)) {As per coding guidelines: "
packages/storage-evm/**: Verify: Deploy scripts are idempotent and environment-aware".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts` around lines 733 - 747, The deploy branch is non-idempotent because it always calls deployCommunity and overwrites state even when a same-chain state exists; update the logic to respect an explicit force gate (e.g., ENV ENERGY_DEMO_FORCE or a 'force' command) so that when state is present and force is false you skip deployment and reuse the existing state; specifically, in the block that checks (command === 'deploy' || (!state && !skipAutoDeploy)) add a condition to detect existing state and only call deployFactoryIfNeeded(), deployCommunity, and saveState when force is true (otherwise log that existing state is being reused), and keep deriveActors/console output but ensure actors are taken from the existing state when skipping deployment.
142-168:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlock random actor fallback for mainnet actor derivation.
Line 164 and Line 167 still allow random address generation, which can orphan ownership/fees on real deployments when env inputs are missing or short.
Suggested guard
-function deriveActors(): Actors { +function deriveActors(isMainnet: boolean): Actors { @@ - if (parts.length < 7) - throw new Error('ENERGY_ACTOR_PRIVATE_KEYS must have at least 7 keys'); + if (isMainnet && parts.length < 10) { + throw new Error( + 'Mainnet requires ENERGY_ACTOR_PRIVATE_KEYS with at least 10 keys (HH1-5, Inv1-2, community, aggregator, gridOperator)', + ); + } @@ - while (addresses.length < 10) addresses.push(Wallet.createRandom().address); + while (addresses.length < 10) { + if (isMainnet) { + throw new Error( + 'Mainnet requires deterministic actors via ENERGY_TEST_MNEMONIC or ENERGY_ACTOR_PRIVATE_KEYS', + ); + } + addresses.push(Wallet.createRandom().address); + } } else { - console.log(' Generating random actor addresses (no signing needed).'); - for (let i = 0; i < 10; i++) addresses.push(Wallet.createRandom().address); + if (isMainnet) { + throw new Error( + 'Mainnet requires ENERGY_TEST_MNEMONIC or ENERGY_ACTOR_PRIVATE_KEYS (10 keys minimum)', + ); + } + console.log(' Generating random actor addresses (no signing needed).'); + for (let i = 0; i < 10; i++) addresses.push(Wallet.createRandom().address); }As per coding guidelines: "
packages/storage-evm/**: Verify: Deploy scripts are idempotent and environment-aware".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts` around lines 142 - 168, The deriveActors function currently falls back to generating random addresses (HDNodeWallet.fromPhrase, Wallet.createRandom, and the while loop filling to 10), which must be disabled for mainnet; instead require a valid ENER GY_TEST_MNEMONIC or ENERGY_ACTOR_PRIVATE_KEYS with enough entries and throw a clear error if missing/insufficient. Update deriveActors so: if mnemonic present, derive exactly the intended set (no random fill); else if keysRaw present, parse and require at least N keys (set to the needed count, e.g., 10) and throw an Error like "ENERGY_ACTOR_PRIVATE_KEYS must contain 10 private keys for mainnet" rather than padding with Wallet.createRandom; remove the final else random-generation branch and replace it with a thrown error instructing the operator to set the env var. Ensure references to HDNodeWallet.fromPhrase, new Wallet(...).address, and Wallet.createRandom are removed or only used when mnemonic/keys are present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/`[lang]/dho/[id]/_components/navigation-tabs.tsx:
- Around line 98-102: The "Energy" tab label is hardcoded; update the tabs data
in navigation-tabs.tsx to use the existing next-intl Common translations instead
of the literal string: add a key like "energy" to the Common translation
namespace and replace the literal 'Energy' with the translated value (use the
same translation accessor used elsewhere in this component, e.g., the t function
or Common translations object), keeping href generated by getDhoPathEnergy(lang
as Locale, id as string) unchanged so only the title is localized.
In `@apps/web/src/app/`[lang]/dho/[id]/_components/select-create-action.tsx:
- Around line 32-58: The three new action items in SelectCreateAction hardcode
English strings; add translation keys for their titles and descriptions in the
existing SelectCreateAction i18n namespace (e.g.,
selectCreateAction.energySharing.title/description,
.registerEnergySource.title/description, .addEnergyMember.title/description),
update the component to use the next-intl translator
(useTranslations('SelectCreateAction')) and replace the literal
title/description values with t('energySharing.title'),
t('energySharing.description'), etc., for all three entries, and ensure any
defaultDurationDays/href/icon/disabled fields remain unchanged.
In `@apps/web/src/app/`[lang]/dho/[id]/@tab/energy/page.tsx:
- Around line 17-22: The page currently renders SpaceTabAccessWrapper even when
findSpaceBySlug(...) returns null; modify the page handler (page.tsx) to
short-circuit and return a 404 when spaceFromDb is falsy by calling Next's
notFound() (or the app router equivalent) before rendering; specifically, after
calling findSpaceBySlug({ slug: id }, { db }) check if spaceFromDb is
null/undefined and invoke notFound() so SpaceTabAccessWrapper is only rendered
when spaceFromDb exists and spaceFromDb.web3SpaceId can be safely used.
In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts:
- Line 47: The code currently converts contract-returned space IDs with
memberWeb3SpaceIds.map(Number) which can silently lose precision for IDs >
Number.MAX_SAFE_INTEGER; instead preserve the original big integer/string
representation and pass those to the DB lookup (e.g., set web3SpaceIds to
memberWeb3SpaceIds.map(id => id.toString()) or keep as BigInt and adapt the
query), and update any downstream lookup logic that expects numeric IDs to
accept string/BigInt values so no lossy Number conversion occurs; locate
memberWeb3SpaceIds and the web3SpaceIds property in the route handler to make
this change.
In `@packages/core/src/energy/client/contracts.ts`:
- Around line 159-178: The function getEnergyPpaFactoryAddress declares chainId
twice causing a syntax error; remove the duplicate declaration by deleting the
second "const chainId = getGovernanceChainId();" (the one after computing
chainScoped) and ensure chainScoped uses the previously defined chainId
variable, then return energyPpaV2FactoryAddress[chainId] as before.
In `@packages/core/src/energy/server/queries.ts`:
- Around line 52-53: Replace the SQL fragment that uses
lower(${energyCommunities.communityProxyAddress}) = ${normalized} with a direct
equality using the column reference (e.g.,
eq(energyCommunities.communityProxyAddress, normalized)) so the database can use
existing btree indexes; apply the same change for the other occurrence
referenced around lines 70-71, ensuring you keep the same normalized variable
and query builder/operator (replace sql`lower(...) = ...` usage with eq(...)
against the raw column).
- Around line 85-89: The upsert currently stores addresses using only
toLowerCase(), which can leave leading/trailing whitespace; update the code that
writes the record (the object with communityProxyAddress, energyTokenAddress,
adminAddress, factoryCommunityId) to normalize each address using
input.X.trim().toLowerCase() for communityProxyAddress, energyTokenAddress and
adminAddress before saving (leave factoryCommunityId as-is), so stored addresses
match the finders that use trim().toLowerCase().
In `@packages/epics/src/agreements/plugins/add-energy-member/plugin.tsx`:
- Around line 12-67: The AddEnergyMemberPlugin currently hardcodes all labels
and placeholders (e.g., "Member Address", "Metadata Hash", "Device IDs (comma
separated)", and the Input placeholders) which violates the i18n rule; update
the component to import and call useTranslations(...) and replace every
user-facing string in the AddEnergyMemberPlugin (inside each FormLabel and Input
placeholder for the fields energyMember.memberAddress,
energyMember.metadataHash, and energyMember.deviceIdsCsv) with translation keys
from the locale bundle (e.g., t('agreements.addEnergyMember.memberAddress')
etc.), ensuring all visible text is read via the t(...) function instead of
literals.
In `@packages/epics/src/agreements/plugins/energy-sharing/plugin.tsx`:
- Around line 16-80: The form's visible strings are hardcoded; replace them with
next-intl translation keys by importing the intl hook (e.g., useTranslations or
useIntl) at the top of the energy-sharing component and use it to render labels
and placeholders for each FormLabel and Input in this file (targets: the
FormField instances for names energySharing.settlementWindow,
energySharing.creditPolicy, energySharing.debtPolicy,
energySharing.effectiveFrom). Create/consume message keys (e.g.,
energySharing.settlementWindow.label, .placeholder,
creditPolicy.label/.placeholder, debtPolicy.label/.placeholder,
effectiveFrom.label) and pass the translated strings into FormLabel and Input
placeholder/value attributes instead of the hardcoded English text.
In `@packages/epics/src/agreements/plugins/register-energy-source/plugin.tsx`:
- Around line 16-100: Replace all hardcoded English labels and placeholders in
the energy source form with i18n translation keys: import your translation hook
(e.g., useTranslation/t) at the top of the component and call
t('agreements.registerEnergySource.<key>') for each visible string used in
FormLabel text and Input placeholder (e.g., keys for sourceId, sourceType,
basePricePerKwh, ownershipToken, deviceIdsCsv and example placeholders). Update
the FormLabel and Input props in the render blocks for the FormField instances
(those with name="energySource.sourceId", "energySource.sourceType",
"energySource.basePricePerKwh", "energySource.ownershipToken",
"energySource.deviceIdsCsv") to use t(...) instead of literal strings and add
corresponding entries to the locale files.
In `@packages/epics/src/governance/components/create-add-energy-member-form.tsx`:
- Around line 26-30: parseDeviceIds currently drops non-numeric tokens which
allows malformed deviceIdsCsv like "1,foo,2" to pass; change parseDeviceIds(csv:
string) to validate every comma-separated token after trimming using
Number(value) and Number.isFinite and if any token is invalid throw a clear
Error (or return null/undefined) instead of filtering them out, so the
caller/validation logic for deviceIdsCsv will fail submission when any entry is
malformed; keep the function name parseDeviceIds and ensure callers handle the
thrown error/nullable return to surface validation feedback.
In `@packages/epics/src/governance/components/create-energy-proposal-form.tsx`:
- Around line 107-109: The code force-casts spaceId when calling createAgreement
(spaceId as number), which can pass undefined/null downstream; update the submit
path in the CreateEnergyProposalForm component to runtime-guard spaceId (e.g.,
if (!spaceId) return/set form error) or make spaceId a required hidden form
field with validation before invoking createAgreement, and ensure web3SpaceId is
only spread when typeof web3SpaceId === 'number'; reference the createAgreement
call and the spaceId/web3SpaceId variables to locate and change the submit
handler and form validation logic.
- Line 73: Remove the unnecessary "as any" casts and use proper generics: call
useLocalizedProposalResolver with the component's generic (e.g.
useLocalizedProposalResolver<TSchema>()) and assign its result to resolver
without casting; pass the form variable (type UseFormReturn<T>) directly into
useResubmitProposalData<T>(form) instead of casting; build the payload object as
the generic T (omit the "as any" cast on payload) so its type is inferred from
the spreads/fields; and pass form directly into FormProvider (no cast) so
FormProvider receives the correct UseFormReturn<T>. Ensure you reference
resolver, useLocalizedProposalResolver, form, useResubmitProposalData, payload,
and FormProvider when applying these changes.
In `@packages/epics/src/governance/components/create-energy-sharing-form.tsx`:
- Around line 41-43: The component create-energy-sharing-form.tsx is using
hardcoded strings for props label, stickyHeaderTitle and
resubmitTemplateSegment; replace those literals with i18n translation keys via
the project's translation helper (e.g., useTranslation()/t) and pass
t('governance.energySharing.label'),
t('governance.energySharing.stickyHeaderTitle') and
t('governance.energySharing.resubmitTemplateSegment') (or the agreed key names)
instead of the raw strings so the form follows the packages/epics i18n
convention; update imports to include the translation hook if missing and ensure
keys are added to the appropriate locale files.
In
`@packages/epics/src/governance/components/create-register-energy-source-form.tsx`:
- Around line 45-47: Replace the hardcoded UI strings in
create-register-energy-source-form.tsx with translation keys: import the
project's translation hook (e.g., useTranslation or t helper used across
packages/epics) and replace the label="Register Energy Source" and
stickyHeaderTitle="Create Register Energy Source Proposal" prop values with
t('epics.registerEnergySource.label') and
t('epics.registerEnergySource.stickyHeaderTitle') (or the agreed key names),
leaving resubmitTemplateSegment unchanged; also add those keys and translations
to the epics translation JSON files so the strings are localizable.
In `@packages/epics/src/governance/components/proposal-energy-proposal-data.tsx`:
- Around line 21-25: The current renderer in entries.map inside the
ProposalEnergyProposalData component flattens non-primitives with String(value),
producing “[object Object]”; update the rendering logic so arrays remain joined
as now but objects and other structured payload fields are preserved—e.g.,
detect typeof value === 'object' (and non-null) and render a JSON fallback via
JSON.stringify(value, null, 2) or use a small recursive renderer to display
nested keys/values instead of String(value); ensure the rendered output is
inside the same <span className="font-medium break-all"> element (or replace it)
so the UI layout remains consistent.
In `@packages/epics/src/governance/utils/energy-proposal-markers.ts`:
- Around line 24-61: Change both parseEnergyProposalMarker and
stripEnergyProposalMarker to locate the appended marker block from the end of
the description instead of the first occurrence: use lastIndexOf(MARKER_START)
and lastIndexOf(MARKER_END) (or otherwise search for MARKER_START from the end)
so you always target the final appended block; verify end > start and trim
surrounding whitespace as before, then parse the JSON or remove the block using
those anchored indices (refer to MARKER_START, MARKER_END,
parseEnergyProposalMarker, and stripEnergyProposalMarker).
In `@packages/epics/src/treasury/components/assets/space-energy-section.tsx`:
- Around line 14-35: Both formatMicro and formatSignedMicro call
Number(formatUnits(...)) which reintroduces IEEE-754 rounding; instead, remove
the Number() conversion and keep the decimal string returned by formatUnits,
then format that string using a string-based/BigInt-aware formatter (e.g.,
implement or reuse a utility that inserts thousands separators and enforces
maximumFractionDigits on the string by splitting integer/fraction parts) so
balances remain exact; update formatMicro, formatSignedMicro, and any callers to
use formatUnits(...) directly and apply the string-based formatting routine
rather than Number().
In `@packages/epics/src/treasury/components/assets/user-energy-section.tsx`:
- Around line 118-123: The component UserEnergySection currently uses hardcoded
UI strings (e.g., inside CardTitle, CardDescription and the metric labels,
button text and helper text across the referenced ranges) — replace all
user-facing literals with next-intl translation keys: import useTranslations
from 'next-intl' at the top of the component, create a translator (e.g., const t
= useTranslations('UserEnergySection') or appropriate namespace), and replace
each literal passed to CardTitle, CardDescription, the metric label props,
Button children, and helper text with t('your.key') calls (choose descriptive
keys like title, description, metric.activeBalance, button.details,
helper.emptyState, etc.) so every string in UserEnergySection comes from the
translator.
- Around line 53-55: Replace the runtime throws when 'client' is missing in
user-energy-section.tsx with graceful guards: remove the "throw new Error('Smart
wallet is not connected.')" occurrences and instead have each click handler (the
onClick handlers in this component that currently check client) early-return
when client is falsy (optionally trigger a user-facing message/toast), and
update the related action buttons to set disabled={!client} so they cannot be
clicked when the smart wallet is disconnected; ensure all instances where
'client' is checked (the current throw sites) are changed to this pattern so the
UI prevents the action rather than throwing.
In `@packages/i18n/src/messages/de.json`:
- Around line 857-860: The German locale file still has three keys in English:
"energySharing", "registerEnergySource", and "addEnergyMember" — replace their
values with proper German translations (suggestions: "Energieaustausch" for
energySharing, "Energiequelle registrieren" for registerEnergySource, and
"Energiemitglied hinzufügen" for addEnergyMember) so de.json remains fully
localized and consistent with other locales.
In `@packages/i18n/src/messages/es.json`:
- Around line 587-590: The Spanish locale contains untranslated English strings
for the energy flow keys; update the values for "energySharing",
"registerEnergySource", and "addEnergyMember" in the messages object so they are
proper Spanish translations (e.g., match style of "investment":
"Inversión")—replace the English strings with appropriate Spanish phrases for
energy sharing, registering an energy source, and adding an energy member.
In `@packages/i18n/src/messages/fr.json`:
- Around line 857-860: The fr.json entries "investment", "energySharing",
"registerEnergySource", and "addEnergyMember" are still English; replace their
values with proper French translations (e.g. "Investissement" already OK for
investment, translate "Energy Sharing", "Register Energy Source", "Add Energy
Member" to French) so the energy proposal flow is fully localized, and ensure
you follow the project's locale message pattern (update the values only—do not
hardcode locale logic or change keys) so the fr locale stays consistent with
other locales.
In `@packages/i18n/src/messages/pt.json`:
- Around line 587-590: The Portuguese locale file contains untranslated keys
"investment", "energySharing", "registerEnergySource", and "addEnergyMember";
update their values in packages/i18n/src/messages/pt.json so they are proper
Portuguese translations (e.g., for UI copy) and mirror the same key names used
in other locales (do not change the keys), ensuring no hardcoded locale strings
are introduced and the messages remain consistent across locales.
In
`@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts`:
- Around line 712-713: Validate the parsed ENERGY_DEMO_LOOP_MS before use:
ensure Number(getEnv('ENERGY_DEMO_LOOP_MS')) yields a finite positive integer
and clamp or fall back to a safe minimum (e.g., 1000 ms) instead of allowing
NaN/0/negative to be used; update the code around the loopMs declaration and any
other uses (see the block using loopMs around lines 768-781) to compute loopMs =
Math.max(MIN_LOOP_MS, Math.floor(parsed)) after checking Number.isFinite(parsed)
and log a warning/error when an invalid value was provided so the loop cannot
spin with effectively zero sleep.
- Around line 709-725: Add a hard fail guard at the start of main() to ensure
the script only runs on Base mainnet: check ethers.provider.getNetwork().chainId
against Base mainnet's chain id (8453) and, unless an explicit override env var
(e.g. ENERGY_DEMO_FORCE === '1') is set, log a clear error and exit early; place
this check immediately after obtaining network (the existing network = await
ethers.provider.getNetwork() call) and before any deploy/run logic that uses
hardcoded Base constants (references: main(), getEnv, ENERGY_DEMO_COMMAND,
skipAutoDeploy).
In `@packages/storage-postgres/migrations/0048_energy_communities.sql`:
- Around line 16-20: The current btree indexes (energy_communities_proxy_uidx on
community_proxy_address and energy_communities_admin_idx on admin_address) don’t
match queries that compare lower(address) and also allow case-variant
duplicates; replace or supplement them with functional indexes using lower(...)
so PostgreSQL can use the index and the unique constraint enforces canonicalized
addresses: create a UNIQUE INDEX on lower(community_proxy_address) (dropping or
replacing the existing energy_communities_proxy_uidx after deduplication) and
create a normal (or unique if required) INDEX on lower(admin_address) (replacing
energy_communities_admin_idx as appropriate); ensure you first canonicalize
existing rows (merge or reject case-variant duplicates) before changing the
unique index to avoid migration failures.
In `@packages/storage-postgres/src/schema/energy-community.ts`:
- Around line 24-25: The schema defines factoryCommunityId using
integer('factory_community_id') which can overflow for on-chain IDs; change the
column to use bigint('factory_community_id') so the DB type matches the
activation data model and TypeScript types; update any related TypeScript
model/type mappings if present to use BigInt/number|string as your codebase
convention for bigint-backed IDs (reference: factoryCommunityId, integer(),
activatedAt).
---
Outside diff comments:
In `@packages/epics/src/proposals/components/proposal-detail.tsx`:
- Around line 1106-1427: The proposal body currently displays the raw energy
marker because MarkdownSuspense is passed content unchanged; update the render
to strip the energy marker before passing to MarkdownSuspense using the same
pattern as Investment/Exchange: when label is one of 'Energy Sharing' |
'Register Energy Source' | 'Add Energy Member' detect/parse via
parseEnergyProposalMarker (or reuse energyMarkerData) and call the corresponding
strip function (e.g., stripEnergyProposalMarker) to produce strippedContent,
then pass strippedContent to MarkdownSuspense (fall back to content when parsing
fails); ensure you reference energyMarkerData, parseEnergyProposalMarker, and
the new/expected stripEnergyProposalMarker in the change.
---
Duplicate comments:
In
`@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts`:
- Around line 320-341: Replace the remaining explicit any usages by using proper
types: change catch (e: any) to catch (e: unknown) and normalize the error into
a message/reason (e.g., if (e instanceof Error) use e.message else String(e));
type receipt.logs as ethers.providers.Log[] and import that type so receipt is
checked as TransactionReceipt; when parsing logs, type the parsed result as
ethers.utils.LogDescription (the return of factory.interface.parseLog) and avoid
any by mapping receipt.logs as Log[] and filtering for (ld?: LogDescription) =>
ld?.name === 'CommunityDeployed'; update references to
factory.interface.parseLog, receipt, logs, and event to use these concrete types
instead of any.
- Around line 413-432: The household rebalancing can oversubscribe shared source
budgets because after computing per-household shares (solarShare, bat1Share,
bat2Share) the code adjusts hhSolar/hhImport without updating remaining global
budgets (solarUsed/importQty), allowing summed hhSolar/hhImport to exceed
solarUsed/importQty; fix by tracking and decrementing remainingSolar and
remainingImport before assigning hhImport or topping hhSolar (use remainingSolar
= solarUsed - hhSolar and remainingImport = importQty - allocatedImport) and
clamp hhImport and hhSolar to those remaining amounts (ensuring hhImport =
min(demand - allocated, remainingImport) and any leftover demand after import is
taken from remainingSolar up to remainingSolar) and finally recompute total and
adjust hhSolar by any tiny residual only if remainingSolar permits so that
summed hh* never exceed original solarUsed/importQty.
- Around line 733-747: The deploy branch is non-idempotent because it always
calls deployCommunity and overwrites state even when a same-chain state exists;
update the logic to respect an explicit force gate (e.g., ENV ENERGY_DEMO_FORCE
or a 'force' command) so that when state is present and force is false you skip
deployment and reuse the existing state; specifically, in the block that checks
(command === 'deploy' || (!state && !skipAutoDeploy)) add a condition to detect
existing state and only call deployFactoryIfNeeded(), deployCommunity, and
saveState when force is true (otherwise log that existing state is being
reused), and keep deriveActors/console output but ensure actors are taken from
the existing state when skipping deployment.
- Around line 142-168: The deriveActors function currently falls back to
generating random addresses (HDNodeWallet.fromPhrase, Wallet.createRandom, and
the while loop filling to 10), which must be disabled for mainnet; instead
require a valid ENER GY_TEST_MNEMONIC or ENERGY_ACTOR_PRIVATE_KEYS with enough
entries and throw a clear error if missing/insufficient. Update deriveActors so:
if mnemonic present, derive exactly the intended set (no random fill); else if
keysRaw present, parse and require at least N keys (set to the needed count,
e.g., 10) and throw an Error like "ENERGY_ACTOR_PRIVATE_KEYS must contain 10
private keys for mainnet" rather than padding with Wallet.createRandom; remove
the final else random-generation branch and replace it with a thrown error
instructing the operator to set the env var. Ensure references to
HDNodeWallet.fromPhrase, new Wallet(...).address, and Wallet.createRandom are
removed or only used when mnemonic/keys are present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 83851d8e-0764-4246-97cd-0f3a294a9a8c
📒 Files selected for processing (49)
apps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/add-energy-member/page.tsxapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/energy-sharing/page.tsxapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/register-energy-source/page.tsxapps/web/src/app/[lang]/dho/[id]/@tab/energy/constants.tsapps/web/src/app/[lang]/dho/[id]/@tab/energy/page.tsxapps/web/src/app/[lang]/dho/[id]/_components/navigation-tabs.tsxapps/web/src/app/[lang]/dho/[id]/_components/plugins.tsxapps/web/src/app/[lang]/dho/[id]/_components/select-create-action.tsxapps/web/src/app/[lang]/profile/[personSlug]/_components/profile-tabs.tsxapps/web/src/app/api/v1/people/[personSlug]/energy/route.tsapps/web/src/app/api/v1/spaces/[spaceSlug]/energy/route.tsdocs/energy-ui-rollout.mdpackages/core/src/client.tspackages/core/src/energy/client/contracts.tspackages/core/src/energy/client/index.tspackages/core/src/energy/index.tspackages/core/src/energy/server/index.tspackages/core/src/energy/server/queries.tspackages/core/src/governance/document-label-badge-keys.tspackages/core/src/server.tspackages/epics/src/agreements/plugins/add-energy-member/plugin.tsxpackages/epics/src/agreements/plugins/energy-sharing/plugin.tsxpackages/epics/src/agreements/plugins/index.tspackages/epics/src/agreements/plugins/register-energy-source/plugin.tsxpackages/epics/src/governance/components/create-add-energy-member-form.tsxpackages/epics/src/governance/components/create-energy-proposal-form.tsxpackages/epics/src/governance/components/create-energy-sharing-form.tsxpackages/epics/src/governance/components/create-register-energy-source-form.tsxpackages/epics/src/governance/components/index.tspackages/epics/src/governance/components/proposal-energy-proposal-data.tsxpackages/epics/src/governance/utils/energy-proposal-markers.tspackages/epics/src/proposals/components/proposal-detail.tsxpackages/epics/src/treasury/components/assets/space-energy-section.tsxpackages/epics/src/treasury/components/assets/user-energy-section.tsxpackages/epics/src/treasury/components/index.tspackages/epics/src/treasury/hooks/index.tspackages/epics/src/treasury/hooks/use-space-energy.tspackages/epics/src/treasury/hooks/use-user-energy.tspackages/epics/src/utils/resubmit-proposal-template.tspackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.jsonpackages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.tspackages/storage-postgres/migrations/0048_energy_communities.sqlpackages/storage-postgres/migrations/meta/_journal.jsonpackages/storage-postgres/src/schema/energy-community.tspackages/storage-postgres/src/schema/index.ts
Comment on lines
+587
to
+590
| "investment": "Investimento", | ||
| "energySharing": "Energy Sharing", | ||
| "registerEnergySource": "Register Energy Source", | ||
| "addEnergyMember": "Add Energy Member" |
Contributor
There was a problem hiding this comment.
Localize the new energy labels.
These new keys are still English in the pt locale, so the energy proposal flow will render mixed-language copy. Please replace them with Portuguese strings. As per coding guidelines, message files must stay consistent across locales and avoid hardcoded locale strings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/i18n/src/messages/pt.json` around lines 587 - 590, The Portuguese
locale file contains untranslated keys "investment", "energySharing",
"registerEnergySource", and "addEnergyMember"; update their values in
packages/i18n/src/messages/pt.json so they are proper Portuguese translations
(e.g., for UI copy) and mirror the same key names used in other locales (do not
change the keys), ensuring no hardcoded locale strings are introduced and the
messages remain consistent across locales.
Comment on lines
+709
to
+725
| async function main(): Promise<void> { | ||
| const command = (getEnv('ENERGY_DEMO_COMMAND') ?? 'loop').toLowerCase(); | ||
| const skipAutoDeploy = getEnv('ENERGY_DEMO_SKIP_DEPLOY') === '1'; | ||
| const loopMs = Number(getEnv('ENERGY_DEMO_LOOP_MS') ?? '45000'); | ||
|
|
||
| const [deployer] = await ethers.getSigners(); | ||
| const network = await ethers.provider.getNetwork(); | ||
| const balance = await ethers.provider.getBalance(deployer.address); | ||
|
|
||
| console.log(SEP); | ||
| console.log('EnergyPPAv2 — Base Mainnet Demo v5'); | ||
| console.log(SEP); | ||
| console.log(` Chain ID : ${network.chainId}`); | ||
| console.log(` Command : ${command}`); | ||
| console.log(` Admin : ${deployer.address}`); | ||
| console.log(` Balance : ${ethers.formatEther(balance)} ETH`); | ||
|
|
Contributor
There was a problem hiding this comment.
Add an explicit Base-mainnet chain guard before any deploy/run path.
This script is Base-mainnet-specific (hardcoded Base constants and naming) but currently runs on any selected network. Fail fast unless an explicit override is provided.
Suggested guard
const [deployer] = await ethers.getSigners();
const network = await ethers.provider.getNetwork();
+ const expectedChainId = BigInt(getEnv('ENERGY_DEMO_CHAIN_ID') ?? '8453');
+ if (network.chainId !== expectedChainId) {
+ throw new Error(
+ `Wrong network: expected chainId ${expectedChainId}, got ${network.chainId}`,
+ );
+ }
const balance = await ethers.provider.getBalance(deployer.address);As per coding guidelines: "packages/storage-evm/**: Verify: Deploy scripts are idempotent and environment-aware".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts`
around lines 709 - 725, Add a hard fail guard at the start of main() to ensure
the script only runs on Base mainnet: check ethers.provider.getNetwork().chainId
against Base mainnet's chain id (8453) and, unless an explicit override env var
(e.g. ENERGY_DEMO_FORCE === '1') is set, log a clear error and exit early; place
this check immediately after obtaining network (the existing network = await
ethers.provider.getNetwork() call) and before any deploy/run logic that uses
hardcoded Base constants (references: main(), getEnv, ENERGY_DEMO_COMMAND,
skipAutoDeploy).
Comment on lines
+712
to
+713
| const loopMs = Number(getEnv('ENERGY_DEMO_LOOP_MS') ?? '45000'); | ||
|
|
Contributor
There was a problem hiding this comment.
Validate ENERGY_DEMO_LOOP_MS to prevent accidental tight transaction loops.
If ENERGY_DEMO_LOOP_MS is invalid (NaN, 0, negative), sleep effectively becomes immediate and can spam transactions in loop mode.
Suggested validation
- const loopMs = Number(getEnv('ENERGY_DEMO_LOOP_MS') ?? '45000');
+ const loopMsRaw = getEnv('ENERGY_DEMO_LOOP_MS') ?? '45000';
+ const loopMs = Number(loopMsRaw);
+ if (!Number.isFinite(loopMs) || loopMs < 1000) {
+ throw new Error(`Invalid ENERGY_DEMO_LOOP_MS: ${loopMsRaw}`);
+ }Also applies to: 768-781
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/storage-evm/scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts`
around lines 712 - 713, Validate the parsed ENERGY_DEMO_LOOP_MS before use:
ensure Number(getEnv('ENERGY_DEMO_LOOP_MS')) yields a finite positive integer
and clamp or fall back to a safe minimum (e.g., 1000 ms) instead of allowing
NaN/0/negative to be used; update the code around the loopMs declaration and any
other uses (see the block using loopMs around lines 768-781) to compute loopMs =
Math.max(MIN_LOOP_MS, Math.floor(parsed)) after checking Number.isFinite(parsed)
and log a warning/error when an invalid value was provided so the loop cannot
spin with effectively zero sleep.
Comment on lines
+16
to
+20
| CREATE UNIQUE INDEX "energy_communities_space_id_uidx" ON "energy_communities" USING btree ("space_id"); | ||
| --> statement-breakpoint | ||
| CREATE UNIQUE INDEX "energy_communities_proxy_uidx" ON "energy_communities" USING btree ("community_proxy_address"); | ||
| --> statement-breakpoint | ||
| CREATE INDEX "energy_communities_admin_idx" ON "energy_communities" USING btree ("admin_address"); |
Contributor
There was a problem hiding this comment.
Align the address indexes with the query shape.
packages/core/src/energy/server/queries.ts compares lower(address) against normalized input, but these are plain btree indexes on the raw text columns. PostgreSQL will not use them for those predicates, and the unique constraint still allows case-variant duplicates if anything bypasses the app-side normalization.
Consider functional indexes on lower(community_proxy_address) / lower(admin_address), or change the queries to compare against the canonical stored value directly.
As per coding guidelines, packages/storage-postgres/migrations/**: migrations must preserve data integrity and should keep performance-sensitive queries indexed correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-postgres/migrations/0048_energy_communities.sql` around
lines 16 - 20, The current btree indexes (energy_communities_proxy_uidx on
community_proxy_address and energy_communities_admin_idx on admin_address) don’t
match queries that compare lower(address) and also allow case-variant
duplicates; replace or supplement them with functional indexes using lower(...)
so PostgreSQL can use the index and the unique constraint enforces canonicalized
addresses: create a UNIQUE INDEX on lower(community_proxy_address) (dropping or
replacing the existing energy_communities_proxy_uidx after deduplication) and
create a normal (or unique if required) INDEX on lower(admin_address) (replacing
energy_communities_admin_idx as appropriate); ensure you first canonicalize
existing rows (merge or reject case-variant duplicates) before changing the
unique index to avoid migration failures.
Comment on lines
+24
to
+25
| factoryCommunityId: integer('factory_community_id'), | ||
| activatedAt: timestamp('activated_at').notNull(), |
Contributor
There was a problem hiding this comment.
Keep factoryCommunityId wide enough for on-chain IDs.
factoryCommunityId is sourced from the factory contract, so narrowing it to integer() risks overflow/truncation once ids grow. Please keep this column bigint-backed to match the activation data model.
As per coding guidelines, packages/storage-postgres/**: TypeScript types align with database schema.
Suggested fix
-import {
- integer,
- pgTable,
- serial,
- text,
- timestamp,
- uniqueIndex,
- index,
-} from 'drizzle-orm/pg-core';
+import {
+ bigint,
+ integer,
+ pgTable,
+ serial,
+ text,
+ timestamp,
+ uniqueIndex,
+ index,
+} from 'drizzle-orm/pg-core';
...
- factoryCommunityId: integer('factory_community_id'),
+ factoryCommunityId: bigint('factory_community_id', { mode: 'number' }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/storage-postgres/src/schema/energy-community.ts` around lines 24 -
25, The schema defines factoryCommunityId using integer('factory_community_id')
which can overflow for on-chain IDs; change the column to use
bigint('factory_community_id') so the DB type matches the activation data model
and TypeScript types; update any related TypeScript model/type mappings if
present to use BigInt/number|string as your codebase convention for
bigint-backed IDs (reference: factoryCommunityId, integer(), activatedAt).
- Added functionality to enable energy communities, allowing users to activate energy features for their spaces. - Updated proposal forms for energy sharing, registering energy sources, and adding energy members to include redirection logic based on energy community status. - Enhanced navigation tabs to conditionally display energy-related options based on community settings. - Updated internationalization files to include translations for the new energy community feature.
- Implemented dynamic form fields for energy sources and members in the EnableEnergyCommunityPlugin, allowing users to add and remove sources easily. - Updated validation schemas to enforce rules for source and member data, ensuring correct input formats and relationships between holders and amounts. - Refactored form handling to parse and structure source and member data appropriately for submission.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
packages/i18n/src/messages/es.json (1)
588-591:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the new energy labels in Spanish.
Lines 588-591 are still English, so the
eslocale will show mixed-language copy.As per coding guidelines: "Message files are consistent across locales (no missing keys)" and "No hardcoded locale strings — use configuration".Suggested patch
- "enableEnergyCommunity": "Enable Energy Community", - "energySharing": "Energy Sharing", - "registerEnergySource": "Register Energy Source", - "addEnergyMember": "Add Energy Member" + "enableEnergyCommunity": "Habilitar comunidad energética", + "energySharing": "Compartir energía", + "registerEnergySource": "Registrar fuente de energía", + "addEnergyMember": "Agregar miembro de energía"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/src/messages/es.json` around lines 588 - 591, The Spanish locale file has untranslated keys ("enableEnergyCommunity", "energySharing", "registerEnergySource", "addEnergyMember") — update packages/i18n/src/messages/es.json to provide Spanish translations for those exact keys (e.g., translate the values for enableEnergyCommunity, energySharing, registerEnergySource, addEnergyMember) so the es locale contains the same keys with Spanish strings rather than English; ensure you follow existing translation style and configuration (no hardcoded locale selection).packages/i18n/src/messages/fr.json (1)
858-861:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the new energy labels in French.
Lines 858-861 are English strings in the
frlocale and should be translated.As per coding guidelines: "Message files are consistent across locales (no missing keys)" and "No hardcoded locale strings — use configuration".Suggested patch
- "enableEnergyCommunity": "Enable Energy Community", - "energySharing": "Energy Sharing", - "registerEnergySource": "Register Energy Source", - "addEnergyMember": "Add Energy Member" + "enableEnergyCommunity": "Activer la communauté énergétique", + "energySharing": "Partage d'énergie", + "registerEnergySource": "Enregistrer une source d'énergie", + "addEnergyMember": "Ajouter un membre énergie"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/src/messages/fr.json` around lines 858 - 861, The fr locale contains untranslated English strings for the keys enableEnergyCommunity, energySharing, registerEnergySource, and addEnergyMember; replace those values with proper French translations, ensure the same keys exist and match across all other locale message files, and remove any hardcoded locale text by routing locale-dependent labels through the i18n configuration/lookup (e.g., ensure the app uses the message keys rather than hardcoded strings).apps/web/src/app/api/v1/people/[personSlug]/energy/route.ts (1)
45-48:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid lossy conversion of on-chain space IDs before DB lookup.
Line 47 uses
memberWeb3SpaceIds.map(Number). Large uint values can exceed JS safe integers and resolve to wrongweb3SpaceIds.Suggested patch
- const spaces = await findAllSpacesByWeb3SpaceIds( + const web3SpaceIds = memberWeb3SpaceIds.map((id) => { + const value = Number(id); + if (!Number.isSafeInteger(value)) { + throw new Error(`Invalid web3 space id: ${id.toString()}`); + } + return value; + }); + + const spaces = await findAllSpacesByWeb3SpaceIds( { - web3SpaceIds: memberWeb3SpaceIds.map(Number), + web3SpaceIds, parentOnly: false, }, { db: appDb }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts around lines 45 - 48, The code is converting large on-chain IDs with memberWeb3SpaceIds.map(Number) which can lose precision; instead preserve the full ID strings (or BigInts) when calling findAllSpacesByWeb3SpaceIds: stop using Number(), pass memberWeb3SpaceIds directly or use memberWeb3SpaceIds.map(String) and ensure the findAllSpacesByWeb3SpaceIds signature/DB query expects string IDs (or accept BigInt) so lookups use the exact on-chain values rather than lossy JS numbers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web/src/app/`[lang]/dho/[id]/@aside/agreements/create/energy-sharing/page.tsx:
- Around line 19-34: Extract the repeated route-guard sequence (reading params,
calling findSpaceBySlug, building successfulUrl/backUrl via getDhoPathAgreements
and PATH_SELECT_CREATE_ACTION, then calling findEnergyCommunityBySpaceId and
redirecting/notFound) into a shared async helper (e.g., loadEnergyCreateGuard)
that returns the resolved space and energyMapping or performs redirects/notFound
internally; replace the inline logic in CreateEnergySharingProposalPage with a
call to that helper and update sibling energy create pages to use the same
helper so the params → findSpaceBySlug → backUrl → findEnergyCommunityBySpaceId
flow is centralized and consistent.
In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/energy/route.ts:
- Around line 87-95: The code unconditionally writes chainId: 8453 and coerces
latestCommunityId with Number(...) which can store incorrect chain metadata or
truncate large IDs; update the upsertEnergyCommunityActivation call to (1)
derive chainId from a safe source (e.g., an env config or a value from
communityRecord) instead of hardcoding 8453, (2) validate latestCommunityId
before persisting by parsing it with a safe parser (e.g., BigInt or parseInt
with base 10) and checking for NaN/invalid values, and (3) bail out or log and
skip the upsert if latestCommunityId is invalid to avoid saving bad metadata;
target symbols: upsertEnergyCommunityActivation, chainId, latestCommunityId,
communityRecord.
In `@packages/epics/src/agreements/plugins/enable-energy-community/plugin.tsx`:
- Around line 56-478: The form contains many hardcoded user-facing strings
(labels and placeholders) in the component (plugin.tsx) — replace them with
translation keys using the i18n hook (e.g., call useTranslations('EnergyPlugin')
at the top and use the returned t function). For each FormLabel and Input
placeholder referenced in the render blocks (notably the fields named
energyCommunityActivation.* and the repeated blocks over sourceFields and
memberFields, plus section titles like "Energy Sources" and "Energy Members"),
swap the literal strings for t('key') calls and add corresponding keys to the
EnergyPlugin namespace; keep existing prop names (e.g., FormLabel, Input,
appendSource, appendMember, removeSource, removeMember) unchanged so only string
literals are replaced with translation lookups.
In
`@packages/epics/src/governance/components/create-enable-energy-community-form.tsx`:
- Around line 182-192: The hardcoded English strings passed to
CreateEnergyProposalForm (props label and stickyHeaderTitle) must be replaced
with translation keys: import/use the existing i18n helper (e.g.,
useTranslation) or pass translation key props expected by
CreateEnergyProposalForm, then replace label="Enable Energy Community" and
stickyHeaderTitle="Create Enable Energy Community Proposal" with
t('epics.enableEnergyCommunity.label') and
t('epics.enableEnergyCommunity.stickyHeaderTitle') (or with corresponding key
props if the form accepts keys), and add the new keys to the epics translation
namespace so the form displays localized text; keep all other props
(schemaCreateEnableEnergyCommunityForm, resubmitTemplateSegment, spaceId,
web3SpaceId, successfulUrl, backUrl, plugin={<EnableEnergyCommunityPlugin />})
unchanged.
In `@packages/epics/src/proposals/components/proposal-detail.tsx`:
- Around line 1116-1126: The current validation accepts arrays because typeof
payload === 'object' is true for arrays, yet you cast payload to Record<string,
unknown> for use with ProposalEnergyProposalData; update the guard to explicitly
reject arrays (e.g. add !Array.isArray(parsed.payload)) before returning. Locate
the block that checks parsed and parsed.payload and change the condition to
ensure parsed.payload is an object, not null, and not an array, then return the
payload cast as Record<string, unknown> as before.
- Around line 1107-1112: The inline string checks for energy-related proposal
labels (the four literals compared against the variable label) should be
replaced with a centralized constant/union to avoid drift; add a shared exported
constant (e.g., ENERGY_PROPOSAL_LABELS: readonly string[] or an enum/union type)
in a common module and import it into proposal-detail.tsx, then replace the
four-branch if with a membership check like
ENERGY_PROPOSAL_LABELS.includes(label). Update any other modules that reference
these same literals to use the new constant to keep labels consistent across the
codebase.
---
Duplicate comments:
In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts:
- Around line 45-48: The code is converting large on-chain IDs with
memberWeb3SpaceIds.map(Number) which can lose precision; instead preserve the
full ID strings (or BigInts) when calling findAllSpacesByWeb3SpaceIds: stop
using Number(), pass memberWeb3SpaceIds directly or use
memberWeb3SpaceIds.map(String) and ensure the findAllSpacesByWeb3SpaceIds
signature/DB query expects string IDs (or accept BigInt) so lookups use the
exact on-chain values rather than lossy JS numbers.
In `@packages/i18n/src/messages/es.json`:
- Around line 588-591: The Spanish locale file has untranslated keys
("enableEnergyCommunity", "energySharing", "registerEnergySource",
"addEnergyMember") — update packages/i18n/src/messages/es.json to provide
Spanish translations for those exact keys (e.g., translate the values for
enableEnergyCommunity, energySharing, registerEnergySource, addEnergyMember) so
the es locale contains the same keys with Spanish strings rather than English;
ensure you follow existing translation style and configuration (no hardcoded
locale selection).
In `@packages/i18n/src/messages/fr.json`:
- Around line 858-861: The fr locale contains untranslated English strings for
the keys enableEnergyCommunity, energySharing, registerEnergySource, and
addEnergyMember; replace those values with proper French translations, ensure
the same keys exist and match across all other locale message files, and remove
any hardcoded locale text by routing locale-dependent labels through the i18n
configuration/lookup (e.g., ensure the app uses the message keys rather than
hardcoded strings).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bf0a3889-f457-4c1c-bc06-d643fef73dde
📒 Files selected for processing (23)
apps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/add-energy-member/page.tsxapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/enable-energy-community/page.tsxapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/energy-sharing/page.tsxapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/register-energy-source/page.tsxapps/web/src/app/[lang]/dho/[id]/@tab/energy/page.tsxapps/web/src/app/[lang]/dho/[id]/_components/navigation-tabs.tsxapps/web/src/app/[lang]/dho/[id]/_components/select-create-action.tsxapps/web/src/app/api/v1/people/[personSlug]/energy/route.tsapps/web/src/app/api/v1/spaces/[spaceSlug]/energy/route.tspackages/core/src/energy/client/contracts.tspackages/core/src/governance/document-label-badge-keys.tspackages/epics/src/agreements/plugins/enable-energy-community/plugin.tsxpackages/epics/src/agreements/plugins/index.tspackages/epics/src/governance/components/create-enable-energy-community-form.tsxpackages/epics/src/governance/components/create-energy-proposal-form.tsxpackages/epics/src/governance/components/index.tspackages/epics/src/proposals/components/proposal-detail.tsxpackages/epics/src/utils/resubmit-proposal-template.tspackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.json
Comment on lines
+19
to
+34
| export default async function CreateEnergySharingProposalPage({ | ||
| params, | ||
| }: PageProps) { | ||
| const { lang, id } = await params; | ||
| const spaceFromDb = await findSpaceBySlug({ slug: id }, { db }); | ||
| if (!spaceFromDb) notFound(); | ||
|
|
||
| const successfulUrl = getDhoPathAgreements(lang as Locale, id); | ||
| const backUrl = `${successfulUrl}${PATH_SELECT_CREATE_ACTION}`; | ||
| const energyMapping = await findEnergyCommunityBySpaceId(spaceFromDb.id, { | ||
| db, | ||
| }); | ||
|
|
||
| if (!energyMapping) { | ||
| redirect(backUrl); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Extract this route guard flow into a shared helper used by all energy create pages.
The same params → findSpaceBySlug → backUrl → findEnergyCommunityBySpaceId gate is duplicated across sibling routes; centralizing it will reduce drift and future branching bugs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/web/src/app/`[lang]/dho/[id]/@aside/agreements/create/energy-sharing/page.tsx
around lines 19 - 34, Extract the repeated route-guard sequence (reading params,
calling findSpaceBySlug, building successfulUrl/backUrl via getDhoPathAgreements
and PATH_SELECT_CREATE_ACTION, then calling findEnergyCommunityBySpaceId and
redirecting/notFound) into a shared async helper (e.g., loadEnergyCreateGuard)
that returns the resolved space and energyMapping or performs redirects/notFound
internally; replace the inline logic in CreateEnergySharingProposalPage with a
call to that helper and update sibling energy create pages to use the same
helper so the params → findSpaceBySlug → backUrl → findEnergyCommunityBySpaceId
flow is centralized and consistent.
Comment on lines
+56
to
+478
| <FormLabel>Admin Address</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="0x..." | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
|
|
||
| <FormField | ||
| control={control} | ||
| name="energyCommunityActivation.stablecoin" | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Stablecoin Address</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="0x..." | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
|
|
||
| <FormField | ||
| control={control} | ||
| name="energyCommunityActivation.gridOperator" | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Grid Operator Address</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="0x..." | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
|
|
||
| <FormField | ||
| control={control} | ||
| name="energyCommunityActivation.communityAddress" | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Community Address (optional)</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="0x..." | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
|
|
||
| <FormField | ||
| control={control} | ||
| name="energyCommunityActivation.aggregatorAddress" | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Aggregator Address (optional)</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="0x..." | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
|
|
||
| <FormField | ||
| control={control} | ||
| name="energyCommunityActivation.exportDeviceId" | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Export Device ID (optional)</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="e.g. 1001" | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
|
|
||
| <FormField | ||
| control={control} | ||
| name="energyCommunityActivation.communityFeeBps" | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Community Fee BPS (optional)</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="e.g. 100" | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
|
|
||
| <FormField | ||
| control={control} | ||
| name="energyCommunityActivation.aggregatorFeeBps" | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Aggregator Fee BPS (optional)</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="e.g. 50" | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
|
|
||
| <FormField | ||
| control={control} | ||
| name="energyCommunityActivation.energyTokenName" | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Energy Token Name</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="e.g. Community Energy Credit" | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
|
|
||
| <FormField | ||
| control={control} | ||
| name="energyCommunityActivation.energyTokenSymbol" | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Energy Token Symbol</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="e.g. CEC" | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
|
|
||
| <div className="md:col-span-2 flex flex-col gap-3 rounded-lg border border-border p-4"> | ||
| <div className="text-1 font-medium">Energy Sources</div> | ||
| {sourceFields.map((sourceField, index) => ( | ||
| <div | ||
| key={sourceField.id} | ||
| className="grid grid-cols-1 gap-3 rounded-md border border-border p-3 md:grid-cols-2" | ||
| > | ||
| <FormField | ||
| control={control} | ||
| name={`energyCommunityActivation.sources.${index}.sourceId`} | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Source ID</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="e.g. SOLAR_A" | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
| <FormField | ||
| control={control} | ||
| name={`energyCommunityActivation.sources.${index}.sourceType`} | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Source Type</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="SOLAR or BATTERY" | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
| <FormField | ||
| control={control} | ||
| name={`energyCommunityActivation.sources.${index}.tokenName`} | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Ownership Token Name</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="e.g. Solar A" | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
| <FormField | ||
| control={control} | ||
| name={`energyCommunityActivation.sources.${index}.tokenSymbol`} | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Ownership Token Symbol</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="e.g. SOLA" | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
| <FormField | ||
| control={control} | ||
| name={`energyCommunityActivation.sources.${index}.basePricePerKwh`} | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Base Price Per kWh</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="integer wei value" | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
| <div /> | ||
| <FormField | ||
| control={control} | ||
| name={`energyCommunityActivation.sources.${index}.holdersCsv`} | ||
| render={({ field }) => ( | ||
| <FormItem className="md:col-span-2"> | ||
| <FormLabel>Holder Addresses (comma separated)</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="0x...,0x..." | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
| <FormField | ||
| control={control} | ||
| name={`energyCommunityActivation.sources.${index}.holderAmountsCsv`} | ||
| render={({ field }) => ( | ||
| <FormItem className="md:col-span-2"> | ||
| <FormLabel> | ||
| Holder Amounts (comma separated, same order) | ||
| </FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="1000000000000000000,2000000000000000000" | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
| <div className="md:col-span-2 flex justify-end"> | ||
| <Button | ||
| type="button" | ||
| variant="ghost" | ||
| onClick={() => removeSource(index)} | ||
| disabled={sourceFields.length <= 1} | ||
| > | ||
| <Cross2Icon /> | ||
| Remove Source | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| ))} | ||
| <div className="flex justify-end"> | ||
| <Button | ||
| type="button" | ||
| variant="ghost" | ||
| onClick={() => | ||
| appendSource({ | ||
| sourceId: '', | ||
| sourceType: '', | ||
| tokenName: '', | ||
| tokenSymbol: '', | ||
| basePricePerKwh: '', | ||
| holdersCsv: '', | ||
| holderAmountsCsv: '', | ||
| }) | ||
| } | ||
| > | ||
| <PlusIcon /> | ||
| Add Source | ||
| </Button> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="md:col-span-2 flex flex-col gap-3 rounded-lg border border-border p-4"> | ||
| <div className="text-1 font-medium">Energy Members (optional)</div> | ||
| {memberFields.map((memberField, index) => ( | ||
| <div | ||
| key={memberField.id} | ||
| className="grid grid-cols-1 gap-3 rounded-md border border-border p-3 md:grid-cols-2" | ||
| > | ||
| <FormField | ||
| control={control} | ||
| name={`energyCommunityActivation.members.${index}.memberAddress`} | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Member Address</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="0x..." | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
| <FormField | ||
| control={control} | ||
| name={`energyCommunityActivation.members.${index}.metadataHash`} | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel>Metadata Hash (bytes32)</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="0x..." | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
| <FormField | ||
| control={control} | ||
| name={`energyCommunityActivation.members.${index}.deviceIdsCsv`} | ||
| render={({ field }) => ( | ||
| <FormItem className="md:col-span-2"> | ||
| <FormLabel>Device IDs (comma separated)</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder="101,102" | ||
| value={field.value ?? ''} | ||
| onChange={field.onChange} | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
| <div className="md:col-span-2 flex justify-end"> | ||
| <Button | ||
| type="button" | ||
| variant="ghost" | ||
| onClick={() => removeMember(index)} | ||
| > | ||
| <Cross2Icon /> | ||
| Remove Member | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| ))} | ||
| <div className="flex justify-end"> | ||
| <Button | ||
| type="button" | ||
| variant="ghost" | ||
| onClick={() => | ||
| appendMember({ | ||
| memberAddress: '', | ||
| metadataHash: '', | ||
| deviceIdsCsv: '', | ||
| }) | ||
| } | ||
| > | ||
| <PlusIcon /> | ||
| Add Member |
Contributor
There was a problem hiding this comment.
Hardcoded user-facing strings violate i18n requirements.
All form labels (e.g., "Admin Address", "Stablecoin Address", "Energy Sources") and placeholder text are hardcoded English strings. Per coding guidelines, all user-facing text in packages/epics/** must use translation keys.
Consider extracting these to a translations namespace (e.g., useTranslations('EnergyPlugin')) and replacing hardcoded strings:
const t = useTranslations('EnergyPlugin');
// ...
<FormLabel>{t('adminAddress')}</FormLabel>
<Input placeholder={t('addressPlaceholder')} ... />This affects approximately 20+ labels and placeholders throughout the component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/epics/src/agreements/plugins/enable-energy-community/plugin.tsx`
around lines 56 - 478, The form contains many hardcoded user-facing strings
(labels and placeholders) in the component (plugin.tsx) — replace them with
translation keys using the i18n hook (e.g., call useTranslations('EnergyPlugin')
at the top and use the returned t function). For each FormLabel and Input
placeholder referenced in the render blocks (notably the fields named
energyCommunityActivation.* and the repeated blocks over sourceFields and
memberFields, plus section titles like "Energy Sources" and "Energy Members"),
swap the literal strings for t('key') calls and add corresponding keys to the
EnergyPlugin namespace; keep existing prop names (e.g., FormLabel, Input,
appendSource, appendMember, removeSource, removeMember) unchanged so only string
literals are replaced with translation lookups.
Comment on lines
+182
to
+192
| return ( | ||
| <CreateEnergyProposalForm<FormValues> | ||
| schema={schemaCreateEnableEnergyCommunityForm} | ||
| label="Enable Energy Community" | ||
| stickyHeaderTitle="Create Enable Energy Community Proposal" | ||
| resubmitTemplateSegment="enable-energy-community" | ||
| spaceId={spaceId} | ||
| web3SpaceId={web3SpaceId} | ||
| successfulUrl={successfulUrl} | ||
| backUrl={backUrl} | ||
| plugin={<EnableEnergyCommunityPlugin />} |
Contributor
There was a problem hiding this comment.
Hardcoded label strings should use translation keys.
The label and stickyHeaderTitle props are hardcoded English strings. Per coding guidelines, user-facing text in packages/epics/** should use translation keys.
+ const t = useTranslations('EnergyProposals');
return (
<CreateEnergyProposalForm<FormValues>
schema={schemaCreateEnableEnergyCommunityForm}
- label="Enable Energy Community"
- stickyHeaderTitle="Create Enable Energy Community Proposal"
+ label={t('enableEnergyCommunity.label')}
+ stickyHeaderTitle={t('enableEnergyCommunity.stickyHeaderTitle')}
resubmitTemplateSegment="enable-energy-community"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return ( | |
| <CreateEnergyProposalForm<FormValues> | |
| schema={schemaCreateEnableEnergyCommunityForm} | |
| label="Enable Energy Community" | |
| stickyHeaderTitle="Create Enable Energy Community Proposal" | |
| resubmitTemplateSegment="enable-energy-community" | |
| spaceId={spaceId} | |
| web3SpaceId={web3SpaceId} | |
| successfulUrl={successfulUrl} | |
| backUrl={backUrl} | |
| plugin={<EnableEnergyCommunityPlugin />} | |
| const t = useTranslations('EnergyProposals'); | |
| return ( | |
| <CreateEnergyProposalForm<FormValues> | |
| schema={schemaCreateEnableEnergyCommunityForm} | |
| label={t('enableEnergyCommunity.label')} | |
| stickyHeaderTitle={t('enableEnergyCommunity.stickyHeaderTitle')} | |
| resubmitTemplateSegment="enable-energy-community" | |
| spaceId={spaceId} | |
| web3SpaceId={web3SpaceId} | |
| successfulUrl={successfulUrl} | |
| backUrl={backUrl} | |
| plugin={<EnableEnergyCommunityPlugin />} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/epics/src/governance/components/create-enable-energy-community-form.tsx`
around lines 182 - 192, The hardcoded English strings passed to
CreateEnergyProposalForm (props label and stickyHeaderTitle) must be replaced
with translation keys: import/use the existing i18n helper (e.g.,
useTranslation) or pass translation key props expected by
CreateEnergyProposalForm, then replace label="Enable Energy Community" and
stickyHeaderTitle="Create Enable Energy Community Proposal" with
t('epics.enableEnergyCommunity.label') and
t('epics.enableEnergyCommunity.stickyHeaderTitle') (or with corresponding key
props if the form accepts keys), and add the new keys to the epics translation
namespace so the form displays localized text; keep all other props
(schemaCreateEnableEnergyCommunityForm, resubmitTemplateSegment, spaceId,
web3SpaceId, successfulUrl, backUrl, plugin={<EnableEnergyCommunityPlugin />})
unchanged.
Comment on lines
+1107
to
+1112
| if ( | ||
| label !== 'Enable Energy Community' && | ||
| label !== 'Energy Sharing' && | ||
| label !== 'Register Energy Source' && | ||
| label !== 'Add Energy Member' | ||
| ) { |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Centralize energy proposal label literals to prevent drift.
The four inline label checks are easy to desync from other label registries/mappings. Move these into a shared constant/union and use membership checks here.
Refactor sketch
+const ENERGY_PROPOSAL_LABELS = new Set([
+ 'Enable Energy Community',
+ 'Energy Sharing',
+ 'Register Energy Source',
+ 'Add Energy Member',
+]);
const energyMarkerData = useMemo(() => {
- if (
- label !== 'Enable Energy Community' &&
- label !== 'Energy Sharing' &&
- label !== 'Register Energy Source' &&
- label !== 'Add Energy Member'
- ) {
+ if (!label || !ENERGY_PROPOSAL_LABELS.has(label)) {
return null;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ( | |
| label !== 'Enable Energy Community' && | |
| label !== 'Energy Sharing' && | |
| label !== 'Register Energy Source' && | |
| label !== 'Add Energy Member' | |
| ) { | |
| const ENERGY_PROPOSAL_LABELS = new Set([ | |
| 'Enable Energy Community', | |
| 'Energy Sharing', | |
| 'Register Energy Source', | |
| 'Add Energy Member', | |
| ]); | |
| const energyMarkerData = useMemo(() => { | |
| if (!label || !ENERGY_PROPOSAL_LABELS.has(label)) { | |
| return null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/epics/src/proposals/components/proposal-detail.tsx` around lines
1107 - 1112, The inline string checks for energy-related proposal labels (the
four literals compared against the variable label) should be replaced with a
centralized constant/union to avoid drift; add a shared exported constant (e.g.,
ENERGY_PROPOSAL_LABELS: readonly string[] or an enum/union type) in a common
module and import it into proposal-detail.tsx, then replace the four-branch if
with a membership check like ENERGY_PROPOSAL_LABELS.includes(label). Update any
other modules that reference these same literals to use the new constant to keep
labels consistent across the codebase.
Comment on lines
+1116
to
+1126
| if ( | ||
| !parsed || | ||
| typeof parsed.payload !== 'object' || | ||
| parsed.payload === null | ||
| ) { | ||
| return null; | ||
| } | ||
| return { | ||
| proposalType: parsed.proposalType, | ||
| payload: parsed.payload as Record<string, unknown>, | ||
| }; |
Contributor
There was a problem hiding this comment.
Tighten payload validation to reject arrays before casting.
Line 1118’s object check also accepts arrays, but Line 1125 casts payload as Record<string, unknown>. Add an array guard before passing data to ProposalEnergyProposalData.
Suggested fix
- if (
- !parsed ||
- typeof parsed.payload !== 'object' ||
- parsed.payload === null
- ) {
+ if (
+ !parsed ||
+ typeof parsed.payload !== 'object' ||
+ parsed.payload === null ||
+ Array.isArray(parsed.payload)
+ ) {
return null;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ( | |
| !parsed || | |
| typeof parsed.payload !== 'object' || | |
| parsed.payload === null | |
| ) { | |
| return null; | |
| } | |
| return { | |
| proposalType: parsed.proposalType, | |
| payload: parsed.payload as Record<string, unknown>, | |
| }; | |
| if ( | |
| !parsed || | |
| typeof parsed.payload !== 'object' || | |
| parsed.payload === null || | |
| Array.isArray(parsed.payload) | |
| ) { | |
| return null; | |
| } | |
| return { | |
| proposalType: parsed.proposalType, | |
| payload: parsed.payload as Record<string, unknown>, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/epics/src/proposals/components/proposal-detail.tsx` around lines
1116 - 1126, The current validation accepts arrays because typeof payload ===
'object' is true for arrays, yet you cast payload to Record<string, unknown> for
use with ProposalEnergyProposalData; update the guard to explicitly reject
arrays (e.g. add !Array.isArray(parsed.payload)) before returning. Locate the
block that checks parsed and parsed.payload and change the condition to ensure
parsed.payload is an object, not null, and not an array, then return the payload
cast as Record<string, unknown> as before.
- Enhanced the EnableEnergyCommunityPlugin to support dynamic addition and removal of energy sources and members. - Updated validation schemas to ensure accurate input for sources and members. - Refactored form handling for better data structuring and submission.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/storage-evm/ENERGY_INTERVAL_DATA_FEED.md`:
- Line 161: The validation text incorrectly refers to "source IDs" while the
payload uses meter_id; update the rule language so it requires that each
production row's meter_id is present in the productionDeviceToSource mapping
(i.e., productionDeviceToSource must map that meter_id to a configured source)
instead of requiring a sourceId field in the row; reference the payload field
meter_id and the mapping productionDeviceToSource in the
ENERGY_INTERVAL_DATA_FEED.md validation bullet so integrators know to validate
meter_id against productionDeviceToSource rather than looking for sourceId.
- Around line 158-159: The doc phrase "Send exactly one interval bucket per
`interval_start`" is ambiguous; update the wording to state explicitly that you
must send one row per meter per `interval_start` per `direction` (i.e., after
aggregating any finer-grained source readings for each meter and direction), so
the expected shape matches the tests/examples (multiple rows for the same
interval_start across different meter_id/direction combinations).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b007f0e7-3ad8-4195-8a26-1a3c55bcfa9b
📒 Files selected for processing (1)
packages/storage-evm/ENERGY_INTERVAL_DATA_FEED.md
Comment on lines
+158
to
+159
| - Send exactly one interval bucket per `interval_start`; aggregate raw readings | ||
| before sending if the source system has finer-grained data. |
Contributor
There was a problem hiding this comment.
Clarify granularity: this currently reads like one row total per timestamp.
Line 158 can be interpreted as a single row per interval_start, which conflicts with your example/test shape (multiple rows per interval across meters). Please make this explicit as one row per meter per interval_start per direction (after upstream aggregation).
Suggested wording
-- Send exactly one interval bucket per `interval_start`; aggregate raw readings
- before sending if the source system has finer-grained data.
+- Send exactly one aggregated row per (`interval_start`, `meter_id`, `direction`);
+ if the source system is finer-grained, aggregate to the 15-minute bucket first.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - Send exactly one interval bucket per `interval_start`; aggregate raw readings | |
| before sending if the source system has finer-grained data. | |
| - Send exactly one aggregated row per (`interval_start`, `meter_id`, `direction`); | |
| if the source system is finer-grained, aggregate to the 15-minute bucket first. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/storage-evm/ENERGY_INTERVAL_DATA_FEED.md` around lines 158 - 159,
The doc phrase "Send exactly one interval bucket per `interval_start`" is
ambiguous; update the wording to state explicitly that you must send one row per
meter per `interval_start` per `direction` (i.e., after aggregating any
finer-grained source readings for each meter and direction), so the expected
shape matches the tests/examples (multiple rows for the same interval_start
across different meter_id/direction combinations).
| - Send exactly one interval bucket per `interval_start`; aggregate raw readings | ||
| before sending if the source system has finer-grained data. | ||
| - Consumption rows must use meter IDs `1`, `2`, `3`, `4`, or `5`. | ||
| - Production rows must map to one of the configured source IDs. |
Contributor
There was a problem hiding this comment.
Validation rule mixes source IDs with row schema fields.
Line 161 says production rows map to configured source IDs, but the payload schema carries meter_id (not sourceId). This is likely to confuse integrators and produce invalid feed rows. Rule should require production meter_id values that are mapped via productionDeviceToSource.
Suggested wording
-- Production rows must map to one of the configured source IDs.
+- Production rows must use a production `meter_id` that is configured in the
+ backend `productionDeviceToSource` map to one of the source IDs above.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/storage-evm/ENERGY_INTERVAL_DATA_FEED.md` at line 161, The
validation text incorrectly refers to "source IDs" while the payload uses
meter_id; update the rule language so it requires that each production row's
meter_id is present in the productionDeviceToSource mapping (i.e.,
productionDeviceToSource must map that meter_id to a configured source) instead
of requiring a sourceId field in the row; reference the payload field meter_id
and the mapping productionDeviceToSource in the ENERGY_INTERVAL_DATA_FEED.md
validation bullet so integrators know to validate meter_id against
productionDeviceToSource rather than looking for sourceId.
…y demo - Introduced `energy-ppav2-rds-loop.ts` script for reading interval data from PostgreSQL and submitting on-chain consumption. - Added new environment variables for RDS configuration, including polling intervals and checkpoint management, enhancing the energy demo's functionality.
Resolve conflicts for PR #2190: combine overview + energy nav, merge i18n labels, sequence energy migration after ecosystem logo (0049). Co-authored-by: Cursor <cursoragent@cursor.com>
…loop Restore stashed WIP: publicnode fallback for base-mainnet when RPC_URL is unset, startup log of resolved RPC host, and RDS loop checkpoint update. Co-authored-by: Cursor <cursoragent@cursor.com>
Preview DBs may already have energy_communities from the pre-merge 0048 migration while Drizzle now applies 0049. Use IF NOT EXISTS and guarded FK creation so migrate succeeds without dropping or altering existing data. Co-authored-by: Cursor <cursoragent@cursor.com>
…dling Added functions to manage energy proposal markers and streamline the markdown rendering for various proposal types. This includes a new utility to check for energy proposal labels and a consolidated method for generating the markdown body, improving code readability and maintainability.
…ce PR comment action Introduced a wait step to mitigate HTTP 408 errors from Neon schema-diff after migrations, improving reliability. Updated the Post Schema Diff Comment action to include the base branch and GitHub token for better integration.
- Refactored the energy path function to import from '@hypha-platform/epics'. - Updated navigation tabs to use localized titles for the energy section. - Enhanced user and profile energy sections to include space titles and slugs. - Added new API responses for community roles and addresses in energy-related routes. - Improved energy overview and user energy components for better data presentation and loading states. - Localized the term "Energy" in multiple languages for better accessibility.
…on schema diff Replaced the wait step with a build step for project artifacts to improve deployment reliability. Added error handling for the Neon schema diff step to log warnings on failure, ensuring the preview deploy continues smoothly. Updated the workflow to better manage environment variables and improve integration with Vercel.
Added a new function `syncEnergyCommunityFromFactory` to streamline the process of linking spaces to energy communities. This function retrieves admin candidates and community details from the energy PPA factory, enhancing the existing energy community mapping logic. Updated the GET handler to utilize this new function, improving code organization and maintainability.
Updated the energy route and contract files to use a constant `ENERGY_PPA_CHAIN_ID` instead of dynamically retrieving the governance chain ID. This change simplifies the logic for obtaining the energy PPA factory address and ensures consistency across the application.
- Introduced a new `deployCommunity` function in the energy contract to facilitate community creation with detailed parameters. - Updated the `useAgreementMutationsWeb3Rpc` and `useCreateAgreementOrchestrator` hooks to support additional on-chain transactions for proposals, allowing for seamless integration with the new community deployment. - Enhanced the `CreateEnableEnergyCommunityForm` to auto-fill the admin address with the space executor and added functionality to build extra transactions for DAO proposals. - Improved the UI components to provide better user guidance and ensure the correct execution context for community activation.
… form management - Enhanced the `syncEnergyCommunityFromFactory` function to include error handling for contract reads, ensuring that failures do not disrupt the synchronization process. - Updated the `CreateEnableEnergyCommunityForm` to validate the executor address and automatically populate the admin field when the address resolves. - Added a reference to the form in `CreateEnergyProposalForm` for better control over form state and values, improving user experience during proposal creation.
…ance error handling - Introduced a `safeRead` function to handle contract reads gracefully, returning `null` on failures instead of throwing errors, which allows for partial data retrieval. - Updated the energy route to utilize `safeRead` for various contract functions, improving resilience against RPC failures and misconfigurations. - Modified the UI components to handle potential `null` values for community fees, balances, and addresses, ensuring a smoother user experience with fallback displays for unavailable data.
The space-energy API endpoint funneled `checkSpaceAccess`'s catch-all 500 (which fires whenever the visibility readContract against the public Base RPC throttles) directly to the client. With the new EnergyPPAv2 community deployed for Paphos, this masked the working on-chain state behind a constant 500 in the UI. Everything the endpoint returns (community proxy, energy token, members, sources, fees) is public on-chain data already readable from Basescan, so on a non-auth 5xx from the access check we now log a warning and fall through to anonymous-public handling. Legitimate 401/403 from checkSpaceAccess are still propagated unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
…heck Server API routes (notably checkSpaceAccess) were importing the @deprecated `publicClient` singleton, which only reads NEXT_PUBLIC_RPC_URL and therefore always hit the rate-limited mainnet.base.org RPC on Vercel previews. Whenever that RPC throttled, the spaces/[slug]/energy endpoint propagated checkSpaceAccess's 500 "Failed to verify access" to the UI and the energy section never rendered. Two changes: - Server-side web3Client now builds a viem fallback transport across RPC_URL → NEXT_PUBLIC_RPC_URL → mainnet.base.org, so API routes always reach the best-available RPC even when only the client URL is provisioned in the environment. - checkSpaceAccess switches off `publicClient` and uses the server-only web3Client for getSpaceVisibility / isMember / getDelegates* / getSpaceDetails / getDelegators. Co-authored-by: Cursor <cursoragent@cursor.com>
The previous change inferred the client's type from a transport that could be either `HttpTransport` or `FallbackTransport`, which made tsc leak a path into viem internals and broke downstream `check-types`: TS2742: The inferred type of 'web3Client' cannot be named without a reference to '.../viem/_types/clients/transports/fallback'. Always wrap the configured transports in `fallback(...)` (viem accepts a single-entry list) and annotate the export as `PublicClient` so the inferred type stays nameable across all packages. Co-authored-by: Cursor <cursoragent@cursor.com>
…ror management - Removed the redundant call to findEnergyCommunityBySpaceId in the EnergyPage component, simplifying the logic. - Enhanced error handling in the findEnergyCommunityBySpaceId function to gracefully handle missing energy_communities table errors, logging warnings instead of throwing. - Updated syncEnergyCommunityFromFactory to return activation details directly when the energy_communities table is missing, improving resilience in the API response. These changes aim to improve the clarity and reliability of energy community data retrieval and synchronization.
- Updated the `useSpaceEnergy` hook to include an `isAuthLoading` state from the authentication context. - Modified the endpoint logic to return null if authentication is still loading, preventing unnecessary API calls and improving performance during user authentication. These changes enhance the efficiency of the energy data retrieval process by ensuring that API requests are only made when the user is fully authenticated.
…nergy communities - Introduced a new entry in the migration journal for version 7, tagged as "0050_energy_communities_authenticated_grants". This entry includes an index and timestamp, enhancing the tracking of changes related to authenticated grants within energy communities.
…onization - Removed the deprecated `getDb` function call and replaced it with a direct reference to the `db` instance in the energy route. - Renamed the `appDb` parameter to `persistDb` in the `syncEnergyCommunityFromFactory` function to clarify its purpose. - Cleaned up unused imports related to database handling, improving code clarity and maintainability. These changes enhance the overall structure of the energy community synchronization logic and ensure consistent database access throughout the API.
- Improved the `syncEnergyCommunityFromFactory` function to better manage error scenarios during synchronization, ensuring that failures are logged and handled gracefully. - Updated the energy community synchronization process to provide clearer feedback on the status of operations, enhancing the overall user experience. - Cleaned up related code for better readability and maintainability. These changes aim to strengthen the reliability of energy community synchronization and improve the clarity of error reporting.
Resolve conflicts: - Keep energy + main additive exports (core client/server, schema index, label keys) - Merge imports in check-space-access and ai-left-panel - Keep typed web3SpaceId plus extraTransactions in agreement orchestrator - Keep after-build Neon schema-diff step in deploy-preview workflow - Renumber energy migrations 0049/0050 -> 0053/0054 after main's 0049-0052 Co-authored-by: Cursor <cursoragent@cursor.com>
- Prevent lossy Number() conversion of on-chain space IDs in energy route - Use direct eq() on normalized address columns (index-friendly) and trim+lowercase on write - Validate device-id CSV in schema and parse strictly - Render structured energy payloads instead of [object Object] - Anchor energy proposal marker parsing to the trailing block (lastIndexOf) - Reject array payloads before casting in proposal detail - Gracefully disable energy actions when smart wallet is unavailable - Translate energy agreement labels in de/es/fr/pt - Widen factoryCommunityId to bigint in schema and migration - Doc/NatSpec cleanups (EnergyToken, README fences, interval data feed) Co-authored-by: Cursor <cursoragent@cursor.com>
The merge conflict resolution dropped the publicClient import that main's sibling-space membership check relies on, breaking web#check-types. Co-authored-by: Cursor <cursoragent@cursor.com>
…ategy Add REC Level 1 base-purpose ranking and Level 2 social allocation to EnergyPPAv2 (owner-settable, getters, events) and wire them through the factory deployCommunity path. Redesign the Enable Energy Community proposal into friendly sections (optimize-for priorities, social allocation, member pickers with meter counts, source ownership %, collapsible advanced) and add a new Change Energy Optimization proposal that calls setOptimizationConfig on the community proxy. Surface the current strategy via the space energy API and render energy proposal payloads as readable rows. Co-authored-by: Cursor <cursoragent@cursor.com>
Deploy a fresh EnergyPPAv2 implementation (0x70729e41…) and EnergyPPAv2Factory (0x5F07320B…) carrying the optimization-strategy CommunityParams, and point energyPpaV2FactoryAddress[8453] at it. Add a FACTORY_ONLY flag to the deploy script so it can deploy just the impl + factory (the community is created via the Enable Energy Community proposal). Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/app/api/v1/people/[personSlug]/energy/route.ts (2)
19-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify the caller before returning profile energy balances.
This accepts any bearer token, then reads balances/debt for the requested
personSlug. Resolve the caller via PrivyClient and require it to matchpersonSlug/person.addressor an explicitly authorized relationship before reading financial data.As per coding guidelines,
apps/web/src/app/api/v1/**/*.tsfinancial/write routes should extractAuthorization: Bearer <token>and verify via PrivyClient for caller identification.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts around lines 19 - 33, The route currently only validates that an authToken exists but does not verify the caller's identity or authorization to access the requested person's financial data. Resolve the caller's identity by using PrivyClient with the extracted authToken, then verify that the caller's address matches the requested personSlug or has an explicitly authorized relationship before allowing access to the person's energy balance data. Add this identity verification and authorization check after extracting the authToken and before calling findPersonBySlug, ensuring unauthorized callers receive a 401 or 403 response as appropriate.Source: Coding guidelines
77-133:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIsolate per-community contract read failures.
A single revert or RPC throttle from any
readContractin thisPromise.allrejects the whole profile endpoint. Use the same safe-read/all-settled pattern as the space route so one broken community does not hide all other balances.Proposed direction
- const communityData = await Promise.all( + const communityResults = await Promise.allSettled( communityMappings.map(async (mapping) => { const communityProxy = mapping.communityProxyAddress as `0x${string}`; const spaceRow = spaces.find((s) => s.id === mapping.spaceId); @@ return { @@ }; }), ); + + const communityData = communityResults + .flatMap((result) => (result.status === 'fulfilled' ? [result.value] : []));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts around lines 77 - 133, The current implementation uses Promise.all inside the communityMappings.map function, which means a single failed readContract call (due to RPC throttle or contract revert) will reject the entire request and prevent returning data for all other communities. Replace Promise.all with Promise.allSettled when gathering the four contract read results (energyCreditBalance, debtInStablecoin, creditInStablecoin, ownerships), then handle the settled results by checking each status and either skipping failed communities or returning them with null values instead of propagating the rejection. This ensures one broken community does not hide balances for all other spaces in the response.
♻️ Duplicate comments (2)
apps/web/src/app/[lang]/dho/[id]/_components/select-create-action.tsx (1)
45-91:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the new energy action copy instead of hardcoding strings.
Lines 45-91 still hardcode user-facing English text (
Enable Energy Community,Energy Sharing Proposal,Register Energy Source,Add Energy Member,Change Energy Optimization+ descriptions). Move these into translation keys and read them viat(...)(same pattern already used for other actions in this component).
As per coding guidelines,apps/web/**must use next-intl integration with no hardcoded user-facing strings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`[lang]/dho/[id]/_components/select-create-action.tsx around lines 45 - 91, The energy action entries (Enable Energy Community, Energy Sharing Proposal, Register Energy Source, Add Energy Member, Change Energy Optimization) contain hardcoded English strings for titles and descriptions that violate the no-hardcoded-user-facing-strings guideline for apps/web. Extract all these title and description strings into translation keys in the appropriate i18n translation file, then replace the hardcoded strings in this component with corresponding t(...) function calls using the new translation keys, following the same pattern already used for other actions in this component.Source: Coding guidelines
apps/web/src/app/api/v1/spaces/[spaceSlug]/energy/route.ts (1)
132-140:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
latestCommunityIdbefore converting it tonumber.
latestCommunityIdis a contractuint256;Number(latestCommunityId)can silently round large values and persist the wrongfactoryCommunityId. This is the remaining factory-ID part of the earlier finding.Proposed fix
+ const factoryCommunityId = Number(latestCommunityId); + if (!Number.isSafeInteger(factoryCommunityId)) { + console.warn( + `[spaces/energy] invalid factory community id ${latestCommunityId.toString()}`, + ); + continue; + } + const activation: EnergyCommunityActivation = { spaceId: input.spaceId, chainId: Number(chainId), communityProxyAddress: communityRecord[0], energyTokenAddress: communityRecord[1], adminAddress: communityRecord[2], - factoryCommunityId: Number(latestCommunityId), + factoryCommunityId, activatedAt: new Date(Number(communityRecord[3]) * 1000), };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/energy/route.ts around lines 132 - 140, The factoryCommunityId field is being assigned by converting latestCommunityId directly to a number using Number(latestCommunityId), but since latestCommunityId is a contract uint256, it can exceed JavaScript's safe integer limits and cause silent data loss through rounding. Validate that latestCommunityId is within safe integer bounds before converting it to a number, or use an alternative approach to handle large numbers. Add validation logic that checks if the value is safe to convert using Number.isSafeInteger() or throw an error if the value exceeds safe bounds to prevent persisting incorrect factoryCommunityId values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/deploy-preview.yml:
- Around line 151-155: The "Post Schema Diff Comment to PR" step with id
schema_diff needs a timeout to prevent it from blocking the preview deploy when
Neon stalls. Add a timeout-minutes parameter to this step (the one using
neondatabase/schema-diff-action@v1) with an appropriate small timeout value.
This ensures that even if the action hangs, it will terminate and allow the
workflow to continue, preserving the intended best-effort behavior for the PR
comment.
- Line 155: The `uses` statement in the neondatabase/schema-diff-action action
uses the broad `v1` tag which violates workflow pinning requirements. Replace
the `@v1` tag with a specific version pin such as `@v1.0.1` (latest as of
January 2025) or `@v1.0.0`, or alternatively pin to a specific commit SHA. This
ensures the workflow uses a fixed, predictable version rather than a moving tag
that could change behavior unexpectedly.
- Around line 146-147: Replace the GitHub expression syntax `${{
env.VERCEL_TOKEN }}` with direct shell variable expansion `$VERCEL_TOKEN` in the
vercel env pull and vercel build commands at lines 146-147. Since VERCEL_TOKEN
is already set in the job environment, use the standard shell variable syntax
(prefixed with single `$`) instead of the GitHub expression syntax to avoid
unnecessary template-injection warnings and maintain consistency with the deploy
step commands at lines 184 and 219.
In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/energy/route.ts:
- Around line 211-212: The first findEnergyCommunityBySpaceId call can throw a
PostgreSQL 42P01 error (table does not exist) before the
syncEnergyCommunityFromFactory fallback is reached, causing a 500 error instead
of the intended on-chain fallback. Wrap the findEnergyCommunityBySpaceId call in
a try-catch block that specifically handles missing-table errors (PostgreSQL
error code 42P01) and allows execution to proceed to the
syncEnergyCommunityFromFactory fallback logic when this error occurs, rather
than propagating the error.
- Around line 200-208: The current code only explicitly returns an error
response for 401 and 403 status codes from checkSpaceAccess, but falls through
and serves the energy payload for all other error statuses. This is a security
issue for non-public spaces where access verification has failed. Modify the
logic in the access check block to return the access.response for any error
status that is not explicitly a success case, rather than only returning for 401
or 403. This ensures that when access verification fails for reasons like RPC
errors or verifier failures, the endpoint fails closed by rejecting the request
instead of falling through to serve the payload anonymously.
In `@apps/web/src/utils/check-space-access.ts`:
- Around line 167-193: The organization sibling checks in the file are using
publicClient for contract reads while direct space checks use web3Client with
server fallback capabilities. Replace all publicClient.readContract calls in the
sibling member, delegate, and space details checks (specifically the calls to
isMemberConfig, getDelegatesForSpace, getSpaceDetails, and getDelegators) with
web3Client.readContract calls to ensure consistent error handling and fallback
behavior across all RPC operations. After making these replacements, remove the
now-unused publicClient import from the file.
- Around line 10-16: Add the `'use server'` directive as the first line of the
check-space-access.ts file, before any imports. This explicitly marks the module
as server-only to establish a clear server boundary, preventing accidental
client-side usage of server-only APIs like getDb, findSelf, web3Client,
findSpaceByWeb3Id, and findAllOrganizationSpacesForNodeById that are imported
from `@hypha-platform/core/server`.
---
Outside diff comments:
In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts:
- Around line 19-33: The route currently only validates that an authToken exists
but does not verify the caller's identity or authorization to access the
requested person's financial data. Resolve the caller's identity by using
PrivyClient with the extracted authToken, then verify that the caller's address
matches the requested personSlug or has an explicitly authorized relationship
before allowing access to the person's energy balance data. Add this identity
verification and authorization check after extracting the authToken and before
calling findPersonBySlug, ensuring unauthorized callers receive a 401 or 403
response as appropriate.
- Around line 77-133: The current implementation uses Promise.all inside the
communityMappings.map function, which means a single failed readContract call
(due to RPC throttle or contract revert) will reject the entire request and
prevent returning data for all other communities. Replace Promise.all with
Promise.allSettled when gathering the four contract read results
(energyCreditBalance, debtInStablecoin, creditInStablecoin, ownerships), then
handle the settled results by checking each status and either skipping failed
communities or returning them with null values instead of propagating the
rejection. This ensures one broken community does not hide balances for all
other spaces in the response.
---
Duplicate comments:
In `@apps/web/src/app/`[lang]/dho/[id]/_components/select-create-action.tsx:
- Around line 45-91: The energy action entries (Enable Energy Community, Energy
Sharing Proposal, Register Energy Source, Add Energy Member, Change Energy
Optimization) contain hardcoded English strings for titles and descriptions that
violate the no-hardcoded-user-facing-strings guideline for apps/web. Extract all
these title and description strings into translation keys in the appropriate
i18n translation file, then replace the hardcoded strings in this component with
corresponding t(...) function calls using the new translation keys, following
the same pattern already used for other actions in this component.
In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/energy/route.ts:
- Around line 132-140: The factoryCommunityId field is being assigned by
converting latestCommunityId directly to a number using
Number(latestCommunityId), but since latestCommunityId is a contract uint256, it
can exceed JavaScript's safe integer limits and cause silent data loss through
rounding. Validate that latestCommunityId is within safe integer bounds before
converting it to a number, or use an alternative approach to handle large
numbers. Add validation logic that checks if the value is safe to convert using
Number.isSafeInteger() or throw an error if the value exceeds safe bounds to
prevent persisting incorrect factoryCommunityId values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c3a5dc8e-cb54-4a2b-ae94-7b3556688908
📒 Files selected for processing (12)
.github/workflows/deploy-preview.ymlapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/change-energy-optimization/page.tsxapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/enable-energy-community/page.tsxapps/web/src/app/[lang]/dho/[id]/@tab/energy/constants.tsapps/web/src/app/[lang]/dho/[id]/@tab/energy/page.tsxapps/web/src/app/[lang]/dho/[id]/_components/navigation-tabs.tsxapps/web/src/app/[lang]/dho/[id]/_components/plugins.tsxapps/web/src/app/[lang]/dho/[id]/_components/select-create-action.tsxapps/web/src/app/[lang]/profile/[personSlug]/_components/profile-tabs.tsxapps/web/src/app/api/v1/people/[personSlug]/energy/route.tsapps/web/src/app/api/v1/spaces/[spaceSlug]/energy/route.tsapps/web/src/utils/check-space-access.ts
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/app/api/v1/people/[personSlug]/energy/route.ts (2)
19-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify the caller before returning profile energy balances.
This accepts any bearer token, then reads balances/debt for the requested
personSlug. Resolve the caller via PrivyClient and require it to matchpersonSlug/person.addressor an explicitly authorized relationship before reading financial data.As per coding guidelines,
apps/web/src/app/api/v1/**/*.tsfinancial/write routes should extractAuthorization: Bearer <token>and verify via PrivyClient for caller identification.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts around lines 19 - 33, The route currently only validates that an authToken exists but does not verify the caller's identity or authorization to access the requested person's financial data. Resolve the caller's identity by using PrivyClient with the extracted authToken, then verify that the caller's address matches the requested personSlug or has an explicitly authorized relationship before allowing access to the person's energy balance data. Add this identity verification and authorization check after extracting the authToken and before calling findPersonBySlug, ensuring unauthorized callers receive a 401 or 403 response as appropriate.Source: Coding guidelines
77-133:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIsolate per-community contract read failures.
A single revert or RPC throttle from any
readContractin thisPromise.allrejects the whole profile endpoint. Use the same safe-read/all-settled pattern as the space route so one broken community does not hide all other balances.Proposed direction
- const communityData = await Promise.all( + const communityResults = await Promise.allSettled( communityMappings.map(async (mapping) => { const communityProxy = mapping.communityProxyAddress as `0x${string}`; const spaceRow = spaces.find((s) => s.id === mapping.spaceId); @@ return { @@ }; }), ); + + const communityData = communityResults + .flatMap((result) => (result.status === 'fulfilled' ? [result.value] : []));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts around lines 77 - 133, The current implementation uses Promise.all inside the communityMappings.map function, which means a single failed readContract call (due to RPC throttle or contract revert) will reject the entire request and prevent returning data for all other communities. Replace Promise.all with Promise.allSettled when gathering the four contract read results (energyCreditBalance, debtInStablecoin, creditInStablecoin, ownerships), then handle the settled results by checking each status and either skipping failed communities or returning them with null values instead of propagating the rejection. This ensures one broken community does not hide balances for all other spaces in the response.
♻️ Duplicate comments (2)
apps/web/src/app/[lang]/dho/[id]/_components/select-create-action.tsx (1)
45-91:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the new energy action copy instead of hardcoding strings.
Lines 45-91 still hardcode user-facing English text (
Enable Energy Community,Energy Sharing Proposal,Register Energy Source,Add Energy Member,Change Energy Optimization+ descriptions). Move these into translation keys and read them viat(...)(same pattern already used for other actions in this component).
As per coding guidelines,apps/web/**must use next-intl integration with no hardcoded user-facing strings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`[lang]/dho/[id]/_components/select-create-action.tsx around lines 45 - 91, The energy action entries (Enable Energy Community, Energy Sharing Proposal, Register Energy Source, Add Energy Member, Change Energy Optimization) contain hardcoded English strings for titles and descriptions that violate the no-hardcoded-user-facing-strings guideline for apps/web. Extract all these title and description strings into translation keys in the appropriate i18n translation file, then replace the hardcoded strings in this component with corresponding t(...) function calls using the new translation keys, following the same pattern already used for other actions in this component.Source: Coding guidelines
apps/web/src/app/api/v1/spaces/[spaceSlug]/energy/route.ts (1)
132-140:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
latestCommunityIdbefore converting it tonumber.
latestCommunityIdis a contractuint256;Number(latestCommunityId)can silently round large values and persist the wrongfactoryCommunityId. This is the remaining factory-ID part of the earlier finding.Proposed fix
+ const factoryCommunityId = Number(latestCommunityId); + if (!Number.isSafeInteger(factoryCommunityId)) { + console.warn( + `[spaces/energy] invalid factory community id ${latestCommunityId.toString()}`, + ); + continue; + } + const activation: EnergyCommunityActivation = { spaceId: input.spaceId, chainId: Number(chainId), communityProxyAddress: communityRecord[0], energyTokenAddress: communityRecord[1], adminAddress: communityRecord[2], - factoryCommunityId: Number(latestCommunityId), + factoryCommunityId, activatedAt: new Date(Number(communityRecord[3]) * 1000), };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/energy/route.ts around lines 132 - 140, The factoryCommunityId field is being assigned by converting latestCommunityId directly to a number using Number(latestCommunityId), but since latestCommunityId is a contract uint256, it can exceed JavaScript's safe integer limits and cause silent data loss through rounding. Validate that latestCommunityId is within safe integer bounds before converting it to a number, or use an alternative approach to handle large numbers. Add validation logic that checks if the value is safe to convert using Number.isSafeInteger() or throw an error if the value exceeds safe bounds to prevent persisting incorrect factoryCommunityId values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/deploy-preview.yml:
- Around line 151-155: The "Post Schema Diff Comment to PR" step with id
schema_diff needs a timeout to prevent it from blocking the preview deploy when
Neon stalls. Add a timeout-minutes parameter to this step (the one using
neondatabase/schema-diff-action@v1) with an appropriate small timeout value.
This ensures that even if the action hangs, it will terminate and allow the
workflow to continue, preserving the intended best-effort behavior for the PR
comment.
- Line 155: The `uses` statement in the neondatabase/schema-diff-action action
uses the broad `v1` tag which violates workflow pinning requirements. Replace
the `@v1` tag with a specific version pin such as `@v1.0.1` (latest as of
January 2025) or `@v1.0.0`, or alternatively pin to a specific commit SHA. This
ensures the workflow uses a fixed, predictable version rather than a moving tag
that could change behavior unexpectedly.
- Around line 146-147: Replace the GitHub expression syntax `${{
env.VERCEL_TOKEN }}` with direct shell variable expansion `$VERCEL_TOKEN` in the
vercel env pull and vercel build commands at lines 146-147. Since VERCEL_TOKEN
is already set in the job environment, use the standard shell variable syntax
(prefixed with single `$`) instead of the GitHub expression syntax to avoid
unnecessary template-injection warnings and maintain consistency with the deploy
step commands at lines 184 and 219.
In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/energy/route.ts:
- Around line 211-212: The first findEnergyCommunityBySpaceId call can throw a
PostgreSQL 42P01 error (table does not exist) before the
syncEnergyCommunityFromFactory fallback is reached, causing a 500 error instead
of the intended on-chain fallback. Wrap the findEnergyCommunityBySpaceId call in
a try-catch block that specifically handles missing-table errors (PostgreSQL
error code 42P01) and allows execution to proceed to the
syncEnergyCommunityFromFactory fallback logic when this error occurs, rather
than propagating the error.
- Around line 200-208: The current code only explicitly returns an error
response for 401 and 403 status codes from checkSpaceAccess, but falls through
and serves the energy payload for all other error statuses. This is a security
issue for non-public spaces where access verification has failed. Modify the
logic in the access check block to return the access.response for any error
status that is not explicitly a success case, rather than only returning for 401
or 403. This ensures that when access verification fails for reasons like RPC
errors or verifier failures, the endpoint fails closed by rejecting the request
instead of falling through to serve the payload anonymously.
In `@apps/web/src/utils/check-space-access.ts`:
- Around line 167-193: The organization sibling checks in the file are using
publicClient for contract reads while direct space checks use web3Client with
server fallback capabilities. Replace all publicClient.readContract calls in the
sibling member, delegate, and space details checks (specifically the calls to
isMemberConfig, getDelegatesForSpace, getSpaceDetails, and getDelegators) with
web3Client.readContract calls to ensure consistent error handling and fallback
behavior across all RPC operations. After making these replacements, remove the
now-unused publicClient import from the file.
- Around line 10-16: Add the `'use server'` directive as the first line of the
check-space-access.ts file, before any imports. This explicitly marks the module
as server-only to establish a clear server boundary, preventing accidental
client-side usage of server-only APIs like getDb, findSelf, web3Client,
findSpaceByWeb3Id, and findAllOrganizationSpacesForNodeById that are imported
from `@hypha-platform/core/server`.
---
Outside diff comments:
In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts:
- Around line 19-33: The route currently only validates that an authToken exists
but does not verify the caller's identity or authorization to access the
requested person's financial data. Resolve the caller's identity by using
PrivyClient with the extracted authToken, then verify that the caller's address
matches the requested personSlug or has an explicitly authorized relationship
before allowing access to the person's energy balance data. Add this identity
verification and authorization check after extracting the authToken and before
calling findPersonBySlug, ensuring unauthorized callers receive a 401 or 403
response as appropriate.
- Around line 77-133: The current implementation uses Promise.all inside the
communityMappings.map function, which means a single failed readContract call
(due to RPC throttle or contract revert) will reject the entire request and
prevent returning data for all other communities. Replace Promise.all with
Promise.allSettled when gathering the four contract read results
(energyCreditBalance, debtInStablecoin, creditInStablecoin, ownerships), then
handle the settled results by checking each status and either skipping failed
communities or returning them with null values instead of propagating the
rejection. This ensures one broken community does not hide balances for all
other spaces in the response.
---
Duplicate comments:
In `@apps/web/src/app/`[lang]/dho/[id]/_components/select-create-action.tsx:
- Around line 45-91: The energy action entries (Enable Energy Community, Energy
Sharing Proposal, Register Energy Source, Add Energy Member, Change Energy
Optimization) contain hardcoded English strings for titles and descriptions that
violate the no-hardcoded-user-facing-strings guideline for apps/web. Extract all
these title and description strings into translation keys in the appropriate
i18n translation file, then replace the hardcoded strings in this component with
corresponding t(...) function calls using the new translation keys, following
the same pattern already used for other actions in this component.
In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/energy/route.ts:
- Around line 132-140: The factoryCommunityId field is being assigned by
converting latestCommunityId directly to a number using
Number(latestCommunityId), but since latestCommunityId is a contract uint256, it
can exceed JavaScript's safe integer limits and cause silent data loss through
rounding. Validate that latestCommunityId is within safe integer bounds before
converting it to a number, or use an alternative approach to handle large
numbers. Add validation logic that checks if the value is safe to convert using
Number.isSafeInteger() or throw an error if the value exceeds safe bounds to
prevent persisting incorrect factoryCommunityId values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c3a5dc8e-cb54-4a2b-ae94-7b3556688908
📒 Files selected for processing (12)
.github/workflows/deploy-preview.ymlapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/change-energy-optimization/page.tsxapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/enable-energy-community/page.tsxapps/web/src/app/[lang]/dho/[id]/@tab/energy/constants.tsapps/web/src/app/[lang]/dho/[id]/@tab/energy/page.tsxapps/web/src/app/[lang]/dho/[id]/_components/navigation-tabs.tsxapps/web/src/app/[lang]/dho/[id]/_components/plugins.tsxapps/web/src/app/[lang]/dho/[id]/_components/select-create-action.tsxapps/web/src/app/[lang]/profile/[personSlug]/_components/profile-tabs.tsxapps/web/src/app/api/v1/people/[personSlug]/energy/route.tsapps/web/src/app/api/v1/spaces/[spaceSlug]/energy/route.tsapps/web/src/utils/check-space-access.ts
🛑 Comments failed to post (7)
.github/workflows/deploy-preview.yml (3)
146-147:
⚠️ Potential issue | 🟡 Minor🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/deploy-preview.yml | sed -n '140,155p'Repository: hypha-dao/hypha-web
Length of output: 876
🏁 Script executed:
grep -n "VERCEL_TOKEN" .github/workflows/deploy-preview.yml | head -20Repository: hypha-dao/hypha-web
Length of output: 734
🏁 Script executed:
cat -n .github/workflows/deploy-preview.yml | sed -n '1,50p'Repository: hypha-dao/hypha-web
Length of output: 1672
Use shell env expansion instead of GitHub expression expansion in
run.
VERCEL_TOKENis already set in the job environment; expanding it as${{ env.VERCEL_TOKEN }}inside shell is unnecessary and triggers template-injection static analysis warnings. Quote the shell variable as done in the deploy step (lines 184, 219). This improves consistency and aligns with shell best practices.Proposed change
- vercel env pull --environment=preview --token=${{ env.VERCEL_TOKEN }} - vercel build --token=${{ env.VERCEL_TOKEN }} + vercel env pull --environment=preview --token="${VERCEL_TOKEN}" + vercel build --token="${VERCEL_TOKEN}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.vercel env pull --environment=preview --token="${VERCEL_TOKEN}" vercel build --token="${VERCEL_TOKEN}"🧰 Tools
🪛 zizmor (1.25.2)
[warning] 146-146: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 147-147: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy-preview.yml around lines 146 - 147, Replace the GitHub expression syntax `${{ env.VERCEL_TOKEN }}` with direct shell variable expansion `$VERCEL_TOKEN` in the vercel env pull and vercel build commands at lines 146-147. Since VERCEL_TOKEN is already set in the job environment, use the standard shell variable syntax (prefixed with single `$`) instead of the GitHub expression syntax to avoid unnecessary template-injection warnings and maintain consistency with the deploy step commands at lines 184 and 219.Sources: Coding guidelines, Linters/SAST tools
151-155:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winBound schema-diff runtime so previews actually continue.
continue-on-erroronly helps after the action exits; if Neon stalls, this step can still block the preview deploy until the job timeout. Add a smalltimeout-minutesto preserve the intended “best-effort PR comment” behavior.Proposed timeout
- name: Post Schema Diff Comment to PR id: schema_diff if: needs.detect-changes.outputs.has_migrations == 'true' + timeout-minutes: 5 continue-on-error: true uses: neondatabase/schema-diff-action@v1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.- name: Post Schema Diff Comment to PR id: schema_diff if: needs.detect-changes.outputs.has_migrations == 'true' timeout-minutes: 5 continue-on-error: true uses: neondatabase/schema-diff-action@v1🧰 Tools
🪛 zizmor (1.25.2)
[error] 155-155: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy-preview.yml around lines 151 - 155, The "Post Schema Diff Comment to PR" step with id schema_diff needs a timeout to prevent it from blocking the preview deploy when Neon stalls. Add a timeout-minutes parameter to this step (the one using neondatabase/schema-diff-action@v1) with an appropriate small timeout value. This ensures that even if the action hangs, it will terminate and allow the workflow to continue, preserving the intended best-effort behavior for the PR comment.
155-155:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Verify available immutable refs/tags before replacing the broad `@v1` action pin. set -euo pipefail # Expect: `v1` resolves to a mutable tag; prefer a commit SHA from this response, # or select a more specific release tag if the repository publishes one. gh api repos/neondatabase/schema-diff-action/git/ref/tags/v1 # Expect: list any more specific v1.x tags that could be used instead of `@v1`. gh api repos/neondatabase/schema-diff-action/git/matching-refs/tags/v1. || trueRepository: hypha-dao/hypha-web
Length of output: 582
🌐 Web query:
neondatabase schema-diff-action releases tags GitHub💡 Result:
The neondatabase/schema-diff-action is a GitHub Action that automates database schema comparisons by posting a diff as a comment on pull requests [1][2]. As of June 2026, the repository has had two releases: - v1.0.0: The initial production release, published on November 27, 2024 [3][4]. - v1.0.1: The latest release, published on January 3, 2025. This release included a new feature to omit the comment when the diff is empty and a fix for version parsing [3][5][6]. You can track all tags and releases directly on the GitHub repository [3]. The action is also listed on the GitHub Marketplace [7] and integrated into Neon's official documentation for CI/CD workflows [8][9][2].
Citations:
- 1: https://github.com/neondatabase/schema-diff-action
- 2: https://neon.com/docs/guides/branching-github-actions
- 3: https://github.com/neondatabase/schema-diff-action/releases
- 4: https://github.com/neondatabase/schema-diff-action/releases/tag/v1.0.0
- 5: https://github.com/neondatabase/schema-diff-action/
- 6: neondatabase/schema-diff-action@v1.0.0...v1.0.1
- 7: https://github.com/marketplace/actions/neon-schema-diff-github-action
- 8: https://neon.com/docs/guides/schema-diff
- 9: https://neon.com/docs/guides/neon-github-integration
Pin
schema-diff-actionto a specific release version or commit SHA.
neondatabase/schema-diff-action@v1is a broad moving tag that violates the workflow pinning rule and can change behavior without a PR. Specific versions are available: use eitherv1.0.1(latest as of January 2025) orv1.0.0, or pin to a commit SHA. Per coding guidelines,.github/**actions must use "pinned versions (SHA or specific version tags)."🧰 Tools
🪛 zizmor (1.25.2)
[error] 155-155: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy-preview.yml at line 155, The `uses` statement in the neondatabase/schema-diff-action action uses the broad `v1` tag which violates workflow pinning requirements. Replace the `@v1` tag with a specific version pin such as `@v1.0.1` (latest as of January 2025) or `@v1.0.0`, or alternatively pin to a specific commit SHA. This ensures the workflow uses a fixed, predictable version rather than a moving tag that could change behavior unexpectedly.Sources: Coding guidelines, Linters/SAST tools
apps/web/src/app/api/v1/spaces/[spaceSlug]/energy/route.ts (2)
200-208:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail closed when access verification fails.
For non-public spaces, a verifier/RPC failure means access could not be established; falling through serves the energy payload anonymously instead of preserving the space transparency boundary.
Proposed fix
const access = await checkSpaceAccess(request, space.web3SpaceId as number); if (!access.hasAccess && access.response) { - const status = access.response.status; - if (status === 401 || status === 403) { - return access.response; - } - console.warn( - `[spaces/energy] checkSpaceAccess returned ${status} — falling through (public on-chain data).`, - ); + return access.response; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const access = await checkSpaceAccess(request, space.web3SpaceId as number); if (!access.hasAccess && access.response) { return access.response; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/energy/route.ts around lines 200 - 208, The current code only explicitly returns an error response for 401 and 403 status codes from checkSpaceAccess, but falls through and serves the energy payload for all other error statuses. This is a security issue for non-public spaces where access verification has failed. Modify the logic in the access check block to return the access.response for any error status that is not explicitly a success case, rather than only returning for 401 or 403. This ensures that when access verification fails for reasons like RPC errors or verifier failures, the endpoint fails closed by rejecting the request instead of falling through to serve the payload anonymously.
211-212:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle the missing
energy_communitiestable on the initial lookup.The first
findEnergyCommunityBySpaceIdcall can throw42P01beforesyncEnergyCommunityFromFactoryreaches its missing-table fallback, turning the intended on-chain fallback into a 500.Proposed fix
- let mapping: EnergyCommunityActivation | null = - await findEnergyCommunityBySpaceId(space.id, { db }); + let mapping: EnergyCommunityActivation | null = null; + try { + mapping = await findEnergyCommunityBySpaceId(space.id, { db }); + } catch (e) { + if (isMissingEnergyCommunitiesTableError(e)) { + console.warn( + '[spaces/energy] energy_communities table is missing; syncing from factory without persisted mapping.', + ); + } else { + throw e; + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/energy/route.ts around lines 211 - 212, The first findEnergyCommunityBySpaceId call can throw a PostgreSQL 42P01 error (table does not exist) before the syncEnergyCommunityFromFactory fallback is reached, causing a 500 error instead of the intended on-chain fallback. Wrap the findEnergyCommunityBySpaceId call in a try-catch block that specifically handles missing-table errors (PostgreSQL error code 42P01) and allows execution to proceed to the syncEnergyCommunityFromFactory fallback logic when this error occurs, rather than propagating the error.apps/web/src/utils/check-space-access.ts (2)
10-16:
⚠️ Potential issue | 🟡 Minor🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Verify whether check-space-access is imported from any Client Component or client-marked module. set -euo pipefail rg -n -C3 --type=ts --type=tsx \ "from ['\"].*check-space-access['\"]|from ['\"].*/utils/check-space-access['\"]|import\\(['\"].*check-space-access['\"]\\)" . # Expect: only server-only consumers such as route handlers/server utilities. # If any importing file starts with 'use client', move this utility behind a server-only path # or split the server-only implementation from shared types.Repository: hypha-dao/hypha-web
Length of output: 91
🏁 Script executed:
rg -n -C3 "from ['\"].*check-space-access['\"]|import ['\"].*check-space-access['\"]" --type tsRepository: hypha-dao/hypha-web
Length of output: 12842
🏁 Script executed:
#!/bin/bash # Find the file first fd -i "check-space-access" . # Then search for imports with correct ripgrep syntax rg "check-space-access" --type ts -C2Repository: hypha-dao/hypha-web
Length of output: 12486
🏁 Script executed:
# Also check the file itself to see its exports and imports cat -n apps/web/src/utils/check-space-access.ts | head -50Repository: hypha-dao/hypha-web
Length of output: 1703
🏁 Script executed:
head -1 apps/web/src/utils/check-space-access.tsRepository: hypha-dao/hypha-web
Length of output: 120
🏁 Script executed:
grep -n "use server" apps/web/src/utils/check-space-access.tsRepository: hypha-dao/hypha-web
Length of output: 45
🏁 Script executed:
wc -l apps/web/src/utils/check-space-access.tsRepository: hypha-dao/hypha-web
Length of output: 108
Add
'use server'directive to this utility file.This module imports server-only APIs (
getDb,findSelf,web3Client,findSpaceByWeb3Id,findAllOrganizationSpacesForNodeById) from@hypha-platform/core/serverbut lacks an explicit server-only boundary marker. Add'use server'as the first line to comply with coding guidelines, which require server-only code to use either the'use server'directive or be exported from a./serverpath. Currently all importers are route handlers (inherently server-only), but the absence of an explicit boundary creates risk for future accidental client-side usage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/utils/check-space-access.ts` around lines 10 - 16, Add the `'use server'` directive as the first line of the check-space-access.ts file, before any imports. This explicitly marks the module as server-only to establish a clear server boundary, preventing accidental client-side usage of server-only APIs like getDb, findSelf, web3Client, findSpaceByWeb3Id, and findAllOrganizationSpacesForNodeById that are imported from `@hypha-platform/core/server`.Source: Coding guidelines
167-193:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
web3Clientfor organization sibling reads too.The direct space checks now use the server fallback RPC client, but the
ORGANISATIONsibling checks still callpublicClient. Because these errors are caught and converted tofalse, a public-client RPC failure can incorrectly deny valid organization access. Use the sameweb3Clienthere and drop thepublicClientimport.Proposed consistency fix
- publicClient, getSpaceVisibility, isMember as isMemberConfig, getSpaceDetails, getDelegatesForSpace, @@ - const siblingMember = await publicClient.readContract( + const siblingMember = await web3Client.readContract( @@ - const siblingDelegates = await publicClient.readContract( + const siblingDelegates = await web3Client.readContract( @@ - const siblingDetails = await publicClient.readContract( + const siblingDetails = await web3Client.readContract( @@ - const siblingDelegators = await publicClient.readContract( + const siblingDelegators = await web3Client.readContract(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const siblingMember = await web3Client.readContract( isMemberConfig({ spaceId: siblingBigInt, memberAddress: userAddress, }), ); if (siblingMember) { return true; } const siblingDelegates = await web3Client.readContract( getDelegatesForSpace({ spaceId: siblingBigInt }), ); const isSiblingDelegate = siblingDelegates.some( (delegate) => delegate.toLowerCase() === userAddress.toLowerCase(), ); if (!isSiblingDelegate) { return false; } const siblingDetails = await web3Client.readContract( getSpaceDetails({ spaceId: siblingBigInt }), ); const [, , , , siblingMembers] = siblingDetails; const siblingDelegators = await web3Client.readContract( getDelegators({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/utils/check-space-access.ts` around lines 167 - 193, The organization sibling checks in the file are using publicClient for contract reads while direct space checks use web3Client with server fallback capabilities. Replace all publicClient.readContract calls in the sibling member, delegate, and space details checks (specifically the calls to isMemberConfig, getDelegatesForSpace, getSpaceDetails, and getDelegators) with web3Client.readContract calls to ensure consistent error handling and fallback behavior across all RPC operations. After making these replacements, remove the now-unused publicClient import from the file.
Collapse the three-priority objective picker into a single primary objective select; remaining priorities are auto-filled into a valid distinct ranking for the contract. Also validate that every energy source owner is listed as a member, preventing silent revenue loss since revenue is only distributed to registered members. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # packages/core/src/governance/client/hooks/useCreateAgreementOrchestrator.ts
…anking
The trailing semicolon inside the .extend({...}) object literal broke
parsing (TS1005 / prettier SyntaxError). Also assert the completeRanking
tuple elements are present to satisfy the EnergyPurposeRanking type.
Co-authored-by: Cursor <cursoragent@cursor.com>
- accept decimal/comma kWh prices and convert to internal units - redesign energy proposal detail card for readability - strip energy proposal marker from document card previews - stop wiring factory to stale RegularSpaceToken impl and validate implementation compatibility; add factory repoint admin script Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/app/[lang]/dho/[id]/@tab/energy/page.tsx (1)
21-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle null
web3SpaceIdbefore casting tonumber.
spaceFromDb.web3SpaceIdcan benull, but line 23 casts it directly tonumber. Ifnull, this yieldsNaNpassed toSpaceTabAccessWrapper, which may cause access-check failures or unexpected behavior.🛡️ Proposed fix
const spaceFromDb = await findSpaceBySlug({ slug: id }, { db }); if (!spaceFromDb) notFound(); + if (spaceFromDb.web3SpaceId == null) { + // Energy requires a web3-enabled space + notFound(); + } return ( <SpaceTabAccessWrapper - spaceId={spaceFromDb.web3SpaceId as number} + spaceId={spaceFromDb.web3SpaceId} spaceSlug={id} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`[lang]/dho/[id]/@tab/energy/page.tsx around lines 21 - 25, The spaceId prop being passed to SpaceTabAccessWrapper is casting spaceFromDb.web3SpaceId directly to number without checking if it's null first, which results in NaN being passed to the component. Add a null check before the SpaceTabAccessWrapper component is rendered to verify that spaceFromDb.web3SpaceId is not null. If it is null, either return an error component, redirect the user, or use a fallback value depending on the expected behavior. Only cast to number after confirming the value exists and is not null.apps/web/src/app/api/v1/people/[personSlug]/energy/route.ts (1)
77-133:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdd error handling for per-community contract reads to prevent cascading failures.
Unlike the sibling
/spaces/.../energyroute which usessafeReadto gracefully handle RPC failures, this route will 500 if any single community's contract read fails. Given the documented "aggressive rate-limiting" on public Base RPC, a transient failure for one community will crash the entire request.Consider wrapping each community's reads in a try/catch and filtering out failed communities, or returning partial data with error indicators.
🛡️ Suggested approach
const communityData = await Promise.all( communityMappings.map(async (mapping) => { + try { const communityProxy = mapping.communityProxyAddress as `0x${string}`; const spaceRow = spaces.find((s) => s.id === mapping.spaceId); const [ energyCreditBalance, debtInStablecoin, creditInStablecoin, ownerships, ] = await Promise.all([ // ... existing reads ]); // ... existing return + } catch (e) { + console.warn( + `[people/energy] Failed to read community ${mapping.communityProxyAddress}:`, + e, + ); + return null; + } }), ); + + const validCommunityData = communityData.filter( + (c): c is NonNullable<typeof c> => c !== null, + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts around lines 77 - 133, The communityMappings.map() async function lacks error handling for the Promise.all block containing the four web3Client.readContract() calls. If any contract read fails, the entire request crashes with a 500 error. Wrap the Promise.all containing the contract read operations in a try/catch block within the map function, and either return null for failed communities to be filtered out later, or return partial data with error indicators so that one community's RPC failure doesn't cascade to the entire response.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/deploy-preview.yml:
- Line 155: Replace the floating version tag `@v1` with a pinned commit SHA for
the neondatabase/schema-diff-action to address supply-chain security concerns.
Update the uses field where neondatabase/schema-diff-action is referenced to use
a specific commit hash (e.g., `@abc1234def5678`) instead of `@v1`, ensuring the
action is locked to a known, immutable version.
- Around line 142-147: The Build Project Artifacts step uses GitHub expression
interpolation `${{ env.VERCEL_TOKEN }}` in the run commands, which interpolates
at workflow parse time. Replace both occurrences of `${{ env.VERCEL_TOKEN }}`
with shell variable references `$VERCEL_TOKEN` in the vercel env pull and vercel
build commands to use safer shell variable referencing and avoid potential
issues with special characters.
In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/energy/route.ts:
- Around line 132-140: The conversion of `latestCommunityId` from `bigint` to
`Number` in the `factoryCommunityId` property of the `EnergyCommunityActivation`
object lacks validation and risks silent precision loss if the value exceeds
`Number.MAX_SAFE_INTEGER`. Before assigning to `factoryCommunityId`, add a guard
check to verify that `latestCommunityId` is within safe integer bounds using
`Number.isSafeInteger()` after converting to Number, or validate the `bigint`
value directly against `BigInt(Number.MAX_SAFE_INTEGER)`. If the value exceeds
the safe limit, throw an appropriate error to prevent data integrity issues
rather than silently truncating the value.
In `@apps/web/src/utils/check-space-access.ts`:
- Around line 167-172: The sibling membership checks in the file are using the
deprecated and insecure publicClient instead of the secure web3Client that is
already imported and used elsewhere in the file. Locate all calls to
publicClient.readContract in the sibling member verification logic (around lines
167, 177, 188, 192) and replace publicClient with web3Client to match the secure
pattern already established in the main access checks throughout the same file
(lines 48, 97, 108, 117, 122). This ensures all contract reads use the secure
server-side client consistently.
---
Outside diff comments:
In `@apps/web/src/app/`[lang]/dho/[id]/@tab/energy/page.tsx:
- Around line 21-25: The spaceId prop being passed to SpaceTabAccessWrapper is
casting spaceFromDb.web3SpaceId directly to number without checking if it's null
first, which results in NaN being passed to the component. Add a null check
before the SpaceTabAccessWrapper component is rendered to verify that
spaceFromDb.web3SpaceId is not null. If it is null, either return an error
component, redirect the user, or use a fallback value depending on the expected
behavior. Only cast to number after confirming the value exists and is not null.
In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts:
- Around line 77-133: The communityMappings.map() async function lacks error
handling for the Promise.all block containing the four web3Client.readContract()
calls. If any contract read fails, the entire request crashes with a 500 error.
Wrap the Promise.all containing the contract read operations in a try/catch
block within the map function, and either return null for failed communities to
be filtered out later, or return partial data with error indicators so that one
community's RPC failure doesn't cascade to the entire response.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 75ca464d-7400-492b-9d69-915ae167509c
📒 Files selected for processing (12)
.github/workflows/deploy-preview.ymlapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/change-energy-optimization/page.tsxapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/enable-energy-community/page.tsxapps/web/src/app/[lang]/dho/[id]/@tab/energy/constants.tsapps/web/src/app/[lang]/dho/[id]/@tab/energy/page.tsxapps/web/src/app/[lang]/dho/[id]/_components/navigation-tabs.tsxapps/web/src/app/[lang]/dho/[id]/_components/plugins.tsxapps/web/src/app/[lang]/dho/[id]/_components/select-create-action.tsxapps/web/src/app/[lang]/profile/[personSlug]/_components/profile-tabs.tsxapps/web/src/app/api/v1/people/[personSlug]/energy/route.tsapps/web/src/app/api/v1/spaces/[spaceSlug]/energy/route.tsapps/web/src/utils/check-space-access.ts
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/app/[lang]/dho/[id]/@tab/energy/page.tsx (1)
21-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle null
web3SpaceIdbefore casting tonumber.
spaceFromDb.web3SpaceIdcan benull, but line 23 casts it directly tonumber. Ifnull, this yieldsNaNpassed toSpaceTabAccessWrapper, which may cause access-check failures or unexpected behavior.🛡️ Proposed fix
const spaceFromDb = await findSpaceBySlug({ slug: id }, { db }); if (!spaceFromDb) notFound(); + if (spaceFromDb.web3SpaceId == null) { + // Energy requires a web3-enabled space + notFound(); + } return ( <SpaceTabAccessWrapper - spaceId={spaceFromDb.web3SpaceId as number} + spaceId={spaceFromDb.web3SpaceId} spaceSlug={id} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`[lang]/dho/[id]/@tab/energy/page.tsx around lines 21 - 25, The spaceId prop being passed to SpaceTabAccessWrapper is casting spaceFromDb.web3SpaceId directly to number without checking if it's null first, which results in NaN being passed to the component. Add a null check before the SpaceTabAccessWrapper component is rendered to verify that spaceFromDb.web3SpaceId is not null. If it is null, either return an error component, redirect the user, or use a fallback value depending on the expected behavior. Only cast to number after confirming the value exists and is not null.apps/web/src/app/api/v1/people/[personSlug]/energy/route.ts (1)
77-133:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdd error handling for per-community contract reads to prevent cascading failures.
Unlike the sibling
/spaces/.../energyroute which usessafeReadto gracefully handle RPC failures, this route will 500 if any single community's contract read fails. Given the documented "aggressive rate-limiting" on public Base RPC, a transient failure for one community will crash the entire request.Consider wrapping each community's reads in a try/catch and filtering out failed communities, or returning partial data with error indicators.
🛡️ Suggested approach
const communityData = await Promise.all( communityMappings.map(async (mapping) => { + try { const communityProxy = mapping.communityProxyAddress as `0x${string}`; const spaceRow = spaces.find((s) => s.id === mapping.spaceId); const [ energyCreditBalance, debtInStablecoin, creditInStablecoin, ownerships, ] = await Promise.all([ // ... existing reads ]); // ... existing return + } catch (e) { + console.warn( + `[people/energy] Failed to read community ${mapping.communityProxyAddress}:`, + e, + ); + return null; + } }), ); + + const validCommunityData = communityData.filter( + (c): c is NonNullable<typeof c> => c !== null, + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts around lines 77 - 133, The communityMappings.map() async function lacks error handling for the Promise.all block containing the four web3Client.readContract() calls. If any contract read fails, the entire request crashes with a 500 error. Wrap the Promise.all containing the contract read operations in a try/catch block within the map function, and either return null for failed communities to be filtered out later, or return partial data with error indicators so that one community's RPC failure doesn't cascade to the entire response.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/deploy-preview.yml:
- Line 155: Replace the floating version tag `@v1` with a pinned commit SHA for
the neondatabase/schema-diff-action to address supply-chain security concerns.
Update the uses field where neondatabase/schema-diff-action is referenced to use
a specific commit hash (e.g., `@abc1234def5678`) instead of `@v1`, ensuring the
action is locked to a known, immutable version.
- Around line 142-147: The Build Project Artifacts step uses GitHub expression
interpolation `${{ env.VERCEL_TOKEN }}` in the run commands, which interpolates
at workflow parse time. Replace both occurrences of `${{ env.VERCEL_TOKEN }}`
with shell variable references `$VERCEL_TOKEN` in the vercel env pull and vercel
build commands to use safer shell variable referencing and avoid potential
issues with special characters.
In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/energy/route.ts:
- Around line 132-140: The conversion of `latestCommunityId` from `bigint` to
`Number` in the `factoryCommunityId` property of the `EnergyCommunityActivation`
object lacks validation and risks silent precision loss if the value exceeds
`Number.MAX_SAFE_INTEGER`. Before assigning to `factoryCommunityId`, add a guard
check to verify that `latestCommunityId` is within safe integer bounds using
`Number.isSafeInteger()` after converting to Number, or validate the `bigint`
value directly against `BigInt(Number.MAX_SAFE_INTEGER)`. If the value exceeds
the safe limit, throw an appropriate error to prevent data integrity issues
rather than silently truncating the value.
In `@apps/web/src/utils/check-space-access.ts`:
- Around line 167-172: The sibling membership checks in the file are using the
deprecated and insecure publicClient instead of the secure web3Client that is
already imported and used elsewhere in the file. Locate all calls to
publicClient.readContract in the sibling member verification logic (around lines
167, 177, 188, 192) and replace publicClient with web3Client to match the secure
pattern already established in the main access checks throughout the same file
(lines 48, 97, 108, 117, 122). This ensures all contract reads use the secure
server-side client consistently.
---
Outside diff comments:
In `@apps/web/src/app/`[lang]/dho/[id]/@tab/energy/page.tsx:
- Around line 21-25: The spaceId prop being passed to SpaceTabAccessWrapper is
casting spaceFromDb.web3SpaceId directly to number without checking if it's null
first, which results in NaN being passed to the component. Add a null check
before the SpaceTabAccessWrapper component is rendered to verify that
spaceFromDb.web3SpaceId is not null. If it is null, either return an error
component, redirect the user, or use a fallback value depending on the expected
behavior. Only cast to number after confirming the value exists and is not null.
In `@apps/web/src/app/api/v1/people/`[personSlug]/energy/route.ts:
- Around line 77-133: The communityMappings.map() async function lacks error
handling for the Promise.all block containing the four web3Client.readContract()
calls. If any contract read fails, the entire request crashes with a 500 error.
Wrap the Promise.all containing the contract read operations in a try/catch
block within the map function, and either return null for failed communities to
be filtered out later, or return partial data with error indicators so that one
community's RPC failure doesn't cascade to the entire response.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 75ca464d-7400-492b-9d69-915ae167509c
📒 Files selected for processing (12)
.github/workflows/deploy-preview.ymlapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/change-energy-optimization/page.tsxapps/web/src/app/[lang]/dho/[id]/@aside/agreements/create/enable-energy-community/page.tsxapps/web/src/app/[lang]/dho/[id]/@tab/energy/constants.tsapps/web/src/app/[lang]/dho/[id]/@tab/energy/page.tsxapps/web/src/app/[lang]/dho/[id]/_components/navigation-tabs.tsxapps/web/src/app/[lang]/dho/[id]/_components/plugins.tsxapps/web/src/app/[lang]/dho/[id]/_components/select-create-action.tsxapps/web/src/app/[lang]/profile/[personSlug]/_components/profile-tabs.tsxapps/web/src/app/api/v1/people/[personSlug]/energy/route.tsapps/web/src/app/api/v1/spaces/[spaceSlug]/energy/route.tsapps/web/src/utils/check-space-access.ts
🛑 Comments failed to post (4)
.github/workflows/deploy-preview.yml (2)
142-147:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrefer shell variables over GitHub expression interpolation in
runsteps.Using
${{ env.VERCEL_TOKEN }}interpolates the value at workflow parse time. While the risk is low (secrets are admin-controlled), referencing via shell variable$VERCEL_TOKENis safer and avoids potential issues with special characters.🛡️ Suggested fix
- name: Build Project Artifacts env: BRANCH_DB_URL: ${{ steps.create-branch.outputs.db_url_with_pooler }} run: | - vercel env pull --environment=preview --token=${{ env.VERCEL_TOKEN }} - vercel build --token=${{ env.VERCEL_TOKEN }} + vercel env pull --environment=preview --token="$VERCEL_TOKEN" + vercel build --token="$VERCEL_TOKEN"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.- name: Build Project Artifacts env: BRANCH_DB_URL: ${{ steps.create-branch.outputs.db_url_with_pooler }} run: | vercel env pull --environment=preview --token="$VERCEL_TOKEN" vercel build --token="$VERCEL_TOKEN"🧰 Tools
🪛 zizmor (1.25.2)
[warning] 146-146: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 147-147: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy-preview.yml around lines 142 - 147, The Build Project Artifacts step uses GitHub expression interpolation `${{ env.VERCEL_TOKEN }}` in the run commands, which interpolates at workflow parse time. Replace both occurrences of `${{ env.VERCEL_TOKEN }}` with shell variable references `$VERCEL_TOKEN` in the vercel env pull and vercel build commands to use safer shell variable referencing and avoid potential issues with special characters.Source: Linters/SAST tools
155-155:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin the Neon schema-diff action to a specific commit SHA.
Using
@v1is a floating tag that can change without notice, potentially introducing supply-chain risk. Per coding guidelines, actions should be pinned to a specific SHA.🔒 Suggested fix
- uses: neondatabase/schema-diff-action@v1 + uses: neondatabase/schema-diff-action@<SHA> # v1Replace
<SHA>with the current commit hash for v1 (e.g.,neondatabase/schema-diff-action@abc1234...).🧰 Tools
🪛 zizmor (1.25.2)
[error] 155-155: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy-preview.yml at line 155, Replace the floating version tag `@v1` with a pinned commit SHA for the neondatabase/schema-diff-action to address supply-chain security concerns. Update the uses field where neondatabase/schema-diff-action is referenced to use a specific commit hash (e.g., `@abc1234def5678`) instead of `@v1`, ensuring the action is locked to a known, immutable version.Sources: Coding guidelines, Linters/SAST tools
apps/web/src/app/api/v1/spaces/[spaceSlug]/energy/route.ts (1)
132-140:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
Number(latestCommunityId)may truncate large on-chain IDs.The
latestCommunityIdis abigintfrom the contract. If it exceedsNumber.MAX_SAFE_INTEGER(~9 quadrillion),Number(...)silently loses precision. While unlikely today, this is a data-integrity risk for long-lived systems. Add a safe-integer guard before persisting.🛡️ Proposed fix
+ const factoryCommunityId = Number(latestCommunityId); + if (!Number.isSafeInteger(factoryCommunityId)) { + console.warn( + `[spaces/energy] factoryCommunityId ${latestCommunityId} exceeds safe integer range`, + ); + continue; + } + const activation: EnergyCommunityActivation = { spaceId: input.spaceId, chainId: Number(chainId), communityProxyAddress: communityRecord[0], energyTokenAddress: communityRecord[1], adminAddress: communityRecord[2], - factoryCommunityId: Number(latestCommunityId), + factoryCommunityId, activatedAt: new Date(Number(communityRecord[3]) * 1000), };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/energy/route.ts around lines 132 - 140, The conversion of `latestCommunityId` from `bigint` to `Number` in the `factoryCommunityId` property of the `EnergyCommunityActivation` object lacks validation and risks silent precision loss if the value exceeds `Number.MAX_SAFE_INTEGER`. Before assigning to `factoryCommunityId`, add a guard check to verify that `latestCommunityId` is within safe integer bounds using `Number.isSafeInteger()` after converting to Number, or validate the `bigint` value directly against `BigInt(Number.MAX_SAFE_INTEGER)`. If the value exceeds the safe limit, throw an appropriate error to prevent data integrity issues rather than silently truncating the value.apps/web/src/utils/check-space-access.ts (1)
167-172:
⚠️ Potential issue | 🔴 Critical🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Check if web3Client and publicClient have different configurations/capabilities # that would justify using different clients in the same file echo "=== web3Client definition ===" rg -n -A 10 'export.*web3Client' --type ts echo -e "\n=== publicClient definition ===" rg -n -A 10 'export.*publicClient' --type tsRepository: hypha-dao/hypha-web
Length of output: 2228
🏁 Script executed:
# Examine the file structure and imports echo "=== Full import section of check-space-access.ts ===" head -50 apps/web/src/utils/check-space-access.ts echo -e "\n=== Check for 'use server' directive ===" head -5 apps/web/src/utils/check-space-access.ts echo -e "\n=== Locate and show publicClient import ===" rg -n 'publicClient' apps/web/src/utils/check-space-access.ts -B 2 -A 2 echo -e "\n=== Locate and show web3Client import ===" rg -n 'web3Client' apps/web/src/utils/check-space-access.ts -B 2 -A 2Repository: hypha-dao/hypha-web
Length of output: 3629
🏁 Script executed:
# Check if publicClient is exported from /server paths and understand the intended usage echo "=== Check what publicClient is exported from /server ===" rg -n 'export.*publicClient' packages/core/src/common/server/ --type ts echo -e "\n=== Check all exports from `@hypha-platform/core/server` ===" find packages/core/src -name 'index.ts' -o -name 'server.ts' | xargs grep -l "export" | head -5 echo -e "\n=== Check server exports barrel file ===" cat packages/core/src/server/index.ts 2>/dev/null || cat packages/core/src/index.ts | grep -A 20 "server" echo -e "\n=== Verify publicClient location and export ===" rg -n 'export.*publicClient' packages/core/src --type tsRepository: hypha-dao/hypha-web
Length of output: 656
🏁 Script executed:
# Check the `@hypha-platform/core` exports structure echo "=== Check common web3 index exports ===" cat packages/core/src/common/web3/index.ts 2>/dev/null || echo "File not found" echo -e "\n=== Check common index exports ===" cat packages/core/src/common/index.ts 2>/dev/null || echo "File not found" echo -e "\n=== Check core/client exports ===" rg -n 'publicClient|web3Client' packages/core/src/client/ --type ts 2>/dev/null echo -e "\n=== Find where `@hypha-platform/core/client` is defined ===" fd 'index.ts' packages/core/src/client/ --exec cat {} \; 2>/dev/null | head -30 echo -e "\n=== Check server/index.ts ===" fd 'index.ts' packages/core/src/server/ --exec cat {} \; 2>/dev/null | head -40Repository: hypha-dao/hypha-web
Length of output: 698
🏁 Script executed:
# Find package.json and check export mappings for `@hypha-platform/core` echo "=== Check package.json exports in core ===" cat packages/core/package.json | jq '.exports' 2>/dev/null | head -40 echo -e "\n=== Check if publicClient is accessible via /client path ===" rg -n 'publicClient' packages/core/src/client/index.ts 2>/dev/null || echo "Not in client index" echo -e "\n=== Verify the entire export chain ===" cat packages/core/package.json | jq '.exports | keys' 2>/dev/nullRepository: hypha-dao/hypha-web
Length of output: 767
🏁 Script executed:
# Check what's exported from src/client.ts and src/server.ts echo "=== Check src/client.ts exports ===" cat packages/core/src/client.ts | head -50 echo -e "\n=== Check src/server.ts exports ===" cat packages/core/src/server.ts | head -50 echo -e "\n=== Find publicClient re-export ===" rg -n 'publicClient' packages/core/src/client.ts packages/core/src/server.tsRepository: hypha-dao/hypha-web
Length of output: 1609
🏁 Script executed:
# Check common/server/index.ts and trace the publicClient export echo "=== Check common/server/index.ts exports ===" cat packages/core/src/common/server/index.ts echo -e "\n=== Verify publicClient is in client.ts import path ===" rg -n 'export.*from.*web3' packages/core/src/client.ts packages/core/src/server.ts echo -e "\n=== Check common/index.ts to confirm publicClient export ===" cat packages/core/src/common/index.tsRepository: hypha-dao/hypha-web
Length of output: 934
🏁 Script executed:
# Confirm check-space-access.ts is server-only and trace where publicClient should come from echo "=== Check if check-space-access.ts is a server action/utility ===" head -40 apps/web/src/utils/check-space-access.ts | tail -35 echo -e "\n=== Verify web3Client comes from common/server/web3-rpc ===" cat packages/core/src/common/server/web3-rpc/index.ts echo -e "\n=== Confirm publicClient source ===" cat packages/core/src/common/web3/public-client.ts | head -20 echo -e "\n=== Check if publicClient is re-exported from server paths ===" rg 'publicClient' packages/core/src/server.ts packages/core/src/common/server/ --type tsRepository: hypha-dao/hypha-web
Length of output: 1868
Critical: Using deprecated
publicClientin server-side authorization checks. Replace withweb3Client.The
publicClientimported from@hypha-platform/core/clientis marked@deprecatedwith the note "Insecure. Use server-side version instead." This file is server-side code (check-space-access.ts uses NextRequest and imports from@hypha-platform/core/server), yet the sibling membership checks (lines 167, 177, 188, 192) use the deprecated client instead of the secureweb3Clientthat is already imported and used correctly in the main access checks (lines 48, 97, 108, 117, 122).Replace all
publicClientcalls in sibling checks withweb3Clientfor consistency and security.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/utils/check-space-access.ts` around lines 167 - 172, The sibling membership checks in the file are using the deprecated and insecure publicClient instead of the secure web3Client that is already imported and used elsewhere in the file. Locate all calls to publicClient.readContract in the sibling member verification logic (around lines 167, 177, 188, 192) and replace publicClient with web3Client to match the secure pattern already established in the main access checks throughout the same file (lines 48, 97, 108, 117, 122). This ensures all contract reads use the secure server-side client consistently.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the EnergyPPAv2 on-chain settlement stack for the community energy demo, plus the off-chain VPP fair-split module and Base mainnet demo scripts. See
packages/storage-evm/README-energy-demo.mdfor the full walkthrough.Contracts (
packages/storage-evm/contracts/)EnergyPPAv2.sol— UUPS settlement contract.consumeEnergy(readings[])charges consumers and credits source owners proportionally, splits revenue (community fee → aggregator fee → ownership holders), tracks a singlegridBalancefor grid import/export, and supports debt settlement and grid-credit claims via stablecoin.EnergyPPAv2Factory.sol— One-tx community deployment: EnergyToken + UUPS proxies for EnergyPPAv2 and each RegularSpaceToken, source registration, ownership-token mint, member adds, and fees.EnergyToken.sol— ERC-20 for positive energy credit balances; mintable/burnable only by the EnergyPPAv2 proxy.VPP module (
packages/storage-evm/vpp/)Off-chain TypeScript library that decides allocation before submitting to chain:
types.ts— interfaces (FairSplitInput,FairSplitResult,SourceInfo,MemberInfo,ConsumptionReading,TraceEvent, …).fair-split.ts— 3-pass fair-split algorithm: (1) allocate each source's production by ownership %, (2) redistribute surplus from over-supplied members to deficit members, (3) import remaining deficit from the grid. Optionaltracecallback.build-readings.ts— convertsFairSplitResulttoConsumptionReading[](Wh→kWh, exports, grid imports).on-chain-reader.ts— reads source ownership and member balances for a deployed community.run-interval.ts— orchestrates a 15-min interval (fetch → fair-split → build readings).index.ts— barrel export.fair-split.test.ts—node:testunit tests covering surplus export, grid import, near-balanced, and cloudy scenarios.Base mainnet scripts (
packages/storage-evm/scripts/base-mainnet-contracts-scripts/)deploy-energy-ppa-v2-factory.ts— standalone factory deployment.energy-ppav2-mainnet-demo.ts— all-in-one: deploys a community then runs randomized consumption intervals (deploy/once/loopviaENERGY_DEMO_COMMAND).energy-ppav2-vpp-loop.ts— VPP fair-split + on-chain settlement loop with detailed trace logging.energy-ppav2-demo-state.json— deployed community state (proxy + token + member addresses).Hardhat tests (
packages/storage-evm/test/)EnergyPPAv2Functional.test.ts— full functional suite (deployment, source registration, member management,consumeEnergy, fee distribution, zero-sum, settlement, grid balance, access control).EnergyPPAv2Gas.test.ts— gas benchmarks forconsumeEnergyat varying batch sizes against Base block limits.Docs
packages/storage-evm/README-energy-demo.md— overview of contracts, VPP, scripts, settlement flow, and demo community layout.Test plan
cd packages/storage-evm && npx tsx --test vpp/fair-split.test.tscd packages/storage-evm && npx hardhat test test/EnergyPPAv2Functional.test.tscd packages/storage-evm && npx hardhat test test/EnergyPPAv2Gas.test.tscd packages/storage-evm && npx hardhat compileENERGY_DEMO_COMMAND=deploy npx hardhat run scripts/base-mainnet-contracts-scripts/energy-ppav2-mainnet-demo.ts --network base-mainnetMade with Cursor
Summary by CodeRabbit
Release Notes