Math Precision (zero-rounding, wrong rounding direction) and Numerical Gap seam 1 (precision x invariant). The footprint is the seat's obligation: suggestedContribution = footprint * rate is what the seat holder is asked to fund, so by the guide's rule it is a debt-like quantity and should round UP (or be kept at full precision), yet _recordJob floors measure * factor / 1e6 on every single record.

Each record silently loses up to (FACTOR_SCALE - 1)/FACTOR_SCALE of a footprint unit, and because the loss is applied per job before accumulation, seat.footprint is not sum(measure_i) * factor / 1e6 but sum(floor(measure_i * factor / 1e6)); the gap grows linearly with the number of records.

At the minimum valid inputs the estimate is exactly zero: with footprintFactor = 1 (the smallest value setFootprintFactor accepts) any measure below 1_000_000 yields footprint 0, the job is still stored and counted (seat.jobs += 1, totalJobs += 1, JobRecorded emitted with footprint 0), and its seat's suggestion, outstanding and coverage all read 0, so the record exists but can never be funded against or covered.

The loss always favours the seat holder against the beneficiary. The registry has no way to re-estimate a record later (void+re-record is refused because the job id stays used), so the truncation is permanent.

Fix options that keep the agreed design: (a) round the estimate up ((measure * factor + FACTOR_SCALE - 1) / FACTOR_SCALE, still bounded by the uint128 check), or (b) accumulate the unscaled product measure * factor in the seat and network totals and divide by FACTOR_SCALE only in the views, so truncation happens once per query instead of once per record.

Math Precision (overflow intermediates) and Boundary (max input). setFootprintFactor accepts any non-zero uint256, and the product uint256(measure) * footprintFactor on line 256 is checked arithmetic, so for factor > (2^256 - 1) / (2^128 - 1) = 2^128 + 1 there exist valid uint128 measures whose product exceeds 2^256 - 1. The multiplication then reverts with the compiler's Panic(0x11) before line 257 can raise FootprintOverflow(jobId).

The README promises 'Every failure is a custom error' and 'footprints that would exceed uint128 revert rather than truncate' with FootprintOverflow; the second holds (it does revert) but the first does not. In recordJobs the panic carries no job id, so an operator cannot tell which entry of the batch failed. No funds or state are at risk (owner-set value, revert is clean); this is a diagnostics/boundary gap only.

Fix: bound the factor in setFootprintFactor (e.g. footprintFactor_ <= type(uint128).max, far above any physical emissions factor) or compute the estimate with Math.mulDiv and compare, so the custom error is always the one raised.

Math Precision (overflow intermediates) and Boundary (max input). setContributionRate accepts any non-zero uint256 and suggestedContribution multiplies the seat's accumulated footprint (a uint256 sum of uint128 values) by it with checked arithmetic; networkCoverageBps does the same on line 343 with totalFootprint * rate.

With a rate at or above 2^256 / footprint the views revert with Panic(0x11), which also takes down outstanding and coverageBps because they call suggestedContribution.

Nothing on-chain depends on these views (funding never reads the rate), so the impact is limited to off-chain readers until the owner lowers the rate; it is reported because the Math Precision guide asks for every a * b with a user-influenced operand to be bounded, and the README documents the rate as 'OFFSETS minor units per footprint unit' without any ceiling.

Fix: cap the rate in setContributionRate (e.g. to 1e27, the whole supply per unit, or to type(uint128).max) so the product cannot exceed 2^256 for any reachable footprint.

Numerical Gap seam 3 (boundary x invariant). The two coverage views are meant to describe the same state: outstanding is what the seat still owes and coverageBps is how much of its suggestion is funded. On the suggested == 0 fast path they disagree: outstanding returns `funded >= 0 ?

0 : ...= 0, i.e. fully covered, whilecoverageBpsreturns the sentinel 0, which is also the value of a seat that has footprint and has funded nothing. A seat whose only records truncated to zero footprint (finding 1) or that funded before any record exists (whichfundexplicitly permits) is therefore indistinguishable, bycoverageBps` alone, from a seat that owes its whole suggestion and has paid nothing.

The README documents 'a seat with no footprint reads as 0', so this is reported as an information item for the author and integrators rather than a defect; the alternative that keeps the documented floor semantics is to return BPS (10_000) when suggested == 0 && funded > 0, or to expose the zero-suggestion case as a separate boolean so integrators do not read it as 0% coverage.

Area: Access Control (escalation/lock-out chain) and Asymmetry (admin-variant diff). Ownable2Step only makes transferOwnership two-step; renounceOwnership() is inherited unchanged from Ownable (lib/openzeppelin-contracts/contracts/access/Ownable.sol:76-78) and sets owner to address(0) in a single call.

The README's operational notes (README.md:119-120) state 'ownership transfer is two-step so a typo cannot strand the registry', and its access-control table lists 'Transfer ownership (new owner must accept)' but never mentions renounce.

After renounce: no setter can ever be called (setSeats/setBeneficiary/setRecorder/setFootprintFactor/setContributionRate all onlyOwner), so (a) if beneficiary was not yet set, fund() reverts forever with NotConfigured('setBeneficiary'); (b) if a recorder was set, that hot key keeps recordJob/recordJobs/voidJob powers forever and can no longer be rotated or revoked, which contradicts README.md:123 'Rotate the recorder with setRecorder if the key is exposed'.

This is a trust-boundary asymmetry: transferOwnership is guarded against mistakes, the more destructive sibling is not. Minimal fix that preserves the design: override renounceOwnership() to revert, or route it through the same pending/accept pattern (e.g. require a pendingOwner == address(0) sentinel set in a prior tx), and document it in the README table.

Area: Trust Gap, seam access x asymmetry (admin-controlled setter whose write moment alters the destination of in-flight value). The owner is trusted to choose the beneficiary and the README documents that 'Changing the beneficiary only affects later fund calls'.

The gap is that a fund() transaction already signed and in the mempool is a 'later call' relative to a setBeneficiary that lands first: the holder's safeTransferFrom (line 299) is sent to whatever to is at execution, and the ContributionFunded event will show the new address, so the holder has no way to express 'I am funding the regenerative project at X'.

This is reported as a trust-assumption gap, not a permission bypass: the actor is the owner key (or whoever compromises it), the victim is any seat holder with a pending fund, and the economic delta is the full amount of that fund.

A minimal fix preserving the design is fund(seatId, amount, expectedBeneficiary) that reverts if beneficiary != expectedBeneficiary (same pattern as pinning a fee recipient), leaving the owner's power to change the beneficiary for future contributions intact. Also note the README's 'Neither owner nor recorder can ... redirect funds already sent' is true only once sent.

The registry documents NotConfigured(setting) as 'names the setter still needed' (README 'Errors', and the error's NatSpec at line 135). _recordJob reuses that error for a different condition: the configured seat NFT answered ownerOf(seatId) with the zero address, which the README 'Assumptions' section says is how a nonexistent seat on a pre-OZ-5-style NFT is handled.

With seats already set, the recorder (or a batch via recordJobs, which reverts the whole batch) is told to call setSeats, a setter that is already in force and that the owner would re-point to no effect. The behaviour (refusing to attribute to a phantom seat) is correct; the error misdirects the operator and makes the existence-vs-configuration cases indistinguishable off-chain.

Minimal fix that preserves intent: revert with a dedicated error such as UnknownSeat(seatId) (or reuse NotSeatHolder-style naming) at this line, and keep NotConfigured("setSeats") for the _requireSeatsConfigured path only.

The registry constructor is OffsetsRegistry(address owner_, address offsets) and the README is the document the manifest step reads to fill contracts[].constructorArgs. The custom-token launch schema only resolves $token, $requester (economics.remainderTo) and $contract:EarlierName in application-contract constructor arguments (the token may additionally use $factory, $poolManager, $launchNumber); there is no $owner placeholder.

A launch.json that copies the README literally would carry an unresolvable string where an address is required and would be refused by admission, or, if the string were passed through unresolved, the OffsetsRegistry constructor would be given a non-address word and the launch would fail to deploy. This is outside the Solidity code and no launch.json exists yet, so nothing is broken on-chain today; it is a defect in the deliverable's deployment instructions.

Fix: state the real value for owner_: either $requester (resolves to economics.remainderTo, 0x556bbff02ea3b61431e2d80e4845a90db4526352, which the README already names as the launch's remainder recipient) or a static lowercase address the requester chooses, and keep $token for offsets.

fund(seatId, amount) always pays whatever beneficiary holds at execution time and takes no expected-recipient argument. The README documents that 'Changing the beneficiary only affects later fund calls' and that the owner is trusted to set it, so this is the intended owner power rather than a bypass, and it is recorded here as a trust assumption per the review rules.

The material risk is that a seat holder who approves and submits a contribution to the published regenerative project has no on-chain way to ensure that project receives it: an owner key (or a compromised owner key) that calls setBeneficiary in the same block ahead of the pending fund receives the OFFSETS, and the registry still books the amount as the seat's coverage. No unprivileged party can trigger it.

If the requester wants to close it without changing the ownership model, a minimal, behaviour-preserving option is an overload fund(seatId, amount, expectedBeneficiary) that reverts when beneficiary != expectedBeneficiary; otherwise document the owner's multisig/timelock as the control, as the README's 'Owner key' item already suggests.

Area: Invariant / Numerical gap (economics). The footprint estimate is computed and floored per job record, and only measure == 0 is rejected; an estimate that rounds to zero is accepted. Any job whose measure * footprintFactor < FACTOR_SCALE (1e6) is stored with footprint = 0 yet still increments seat.jobs and totalJobs.

Because the sum of floors is below the floor of the sum, the same work split across many records produces strictly less footprint than one aggregated record, and at the extreme produces none.

The economic consequence is that suggestedContribution, outstanding, coverageBps and networkCoverageBps all derive from this under-estimate: a seat with a thousand live jobs can read a suggested contribution of 0 OFFSETS and a coverage of 0 even after funding (coverage of a zero-footprint seat is defined as 0), and the network total is short by up to one footprint unit per record.

The README states footprints 'that would exceed uint128 revert rather than truncate' but says nothing about the low end, and no test covers it. The recorder is trusted for honest measures, but the loss here is deterministic in the formula and independent of recorder honesty: it depends only on the unit choice (e.g. a factor below 1e6 such as 0.5 gCO2e per unit, or per-second measures with a per-hour factor).

Minimal fix preserving the design: reject estimates that round to zero (if (estimate == 0) revert ZeroValue();) so the recorder must aggregate, or compute the estimate with round-half-up / carry the remainder per seat. Alternatively document that the recorder must pick units such that every record satisfies measure * footprintFactor >= 1e6.

Area: Economic security / Flow gap (execution x first principles), documented here as a privileged-power risk with a race amplifier rather than as an access bypass. fund(seatId, amount) reads the owner-set beneficiary at execution time and transfers the holder's OFFSETS there.

The holder has no way to state which beneficiary they are contributing to, so a setBeneficiary that lands between the holder's approve/fund submission and its inclusion (an ordinary owner reconfiguration, a compromised owner key front-running a large contribution, or a pending two-step ownership acceptance by a new owner) redirects the whole amount.

The README claims 'Neither owner nor recorder can move anyone's OFFSETS ... or redirect funds already sent' and 'Changing the beneficiary only affects later fund calls'; both are true for funds already settled, but a contribution the holder has signed and broadcast is not protected, and the holder's tokens end at a party they did not choose.

This is the only path in the system by which a privileged actor influences where a holder's tokens go, so it is the material trust assumption of the design; the owner role itself is intended and is not the defect. Impact is bounded to the amount of the single contribution and requires owner action, hence low.

Minimal fix that keeps the owner's power to change the beneficiary: add an expectedBeneficiary parameter to fund and revert if it differs from beneficiary (or add an overload that pins it), and amend the README sentence to describe the race.

Area: Invariant (view/aggregate divergence). networkCoverageBps divides totalFunded by totalFootprint * rate. totalFunded includes amounts funded above a seat's own suggestion and amounts funded against seats with no footprint at all (both explicitly allowed by fund), so one seat's over-funding is counted as covering other seats' footprint.

The per-seat metric is not transitive to the network metric: the network can read 10_000 (fully covered) while a seat has coverageBps == 0 and a non-zero outstanding. If the intended meaning of the network figure is 'share of the network's estimated footprint that has been funded against', this is a presentation defect of the aggregate, not a loss of funds; no state or tokens are affected.

If pooling is intended, documenting it in the README avoids a reader taking 10_000 as 'every seat is covered'. A non-pooled version would need the registry to track min(funded, suggested) per seat, which changes retroactively with the rate, so a documentation fix is the minimal one.

_recordJob computes uint256(measure) * footprintFactor with checked arithmetic and only then compares the estimate against type(uint128).max. footprintFactor is an unbounded uint256 the owner may set to any non-zero value, so for factor > type(uint256).max / measure the multiplication itself reverts with Panic(0x11) before the typed FootprintOverflow(jobId) check runs.

The call still reverts and no state changes, so this is not an exploitable defect; the only consequence is that the revert reason is the generic arithmetic panic instead of the documented custom error, which the README lists as the error for footprints that exceed uint128.

The existing test test_recordRejectsFootprintOverflow and the fuzz test testFuzz_estimateIsMeasureTimesFactor only cover factors small enough for the product to fit. test/OffsetsRegistryEdge.t.sol::test_absurdFactorStillRevertsAndRecordsNothing asserts the safe part (revert, nothing recorded) without asserting the panic as the intended error.

Merged from audit_permissions, audit_flow and audit_economics (same root cause, same line). fund(seatId, amount) reads the owner-set beneficiary at line 291 and transfers the holder's OFFSETS there at line 299; there is no expected-recipient argument.

The owner role is intended and this is recorded as a trust assumption with a race amplifier, not a permission bypass: the actor is the owner key (or a compromised one, or a newly accepted owner), the victim is any seat holder whose approve/fund is in flight, and the loss is the full amount of that one contribution.

The README says 'Changing the beneficiary only affects later fund calls' and 'Neither owner nor recorder can ... redirect funds already sent', which is true only for settled funds; a signed, broadcast fund is not protected. Minimal fix preserving the design: fund(seatId, amount, expectedBeneficiary) (or an overload) that reverts when beneficiary != expectedBeneficiary, and a README sentence describing the race. Reproduced in test/scratch/Review.t.sol::test_beneficiaryRedirect.

Merged from audit_math and audit_economics. Only measure == 0 is rejected (line 251); an estimate that floors to zero is accepted and stored, incrementing seat.jobs and totalJobs while adding nothing to footprint.

Because the floor is applied per record before accumulation, seat.footprint == sum(floor(measure_i * factor / 1e6)) rather than floor(sum(measure_i) * factor / 1e6); the gap grows by up to one footprint unit per record and is permanent (void + re-record of the same job id is refused). suggestedContribution, outstanding, coverageBps and networkCoverageBps all derive from the under-estimate, always in the seat holder's favour against the beneficiary.

The author's test test_tinyMeasureRoundsToZeroFootprintButStillCounts pins the zero case as 'documented behaviour', but the README documents only the high end ('footprints that would exceed uint128 revert'). Minimal fixes keeping the design: revert ZeroValue() when estimate == 0 so the recorder must aggregate; or round the estimate up; or accumulate the unscaled product measure * factor and divide by FACTOR_SCALE only in the views.

Reproduced in test/scratch/Review.t.sol::test_roundingToZeroAndDrift.

From audit_permissions. Ownable2Step overrides only transferOwnership; renounceOwnership is inherited unchanged from Ownable (lib/openzeppelin-contracts/contracts/access/Ownable.sol:76-78) and sets owner to address(0) in one call. The README's operational notes say 'ownership transfer is two-step so a typo cannot strand the registry' and tell the operator to 'Rotate the recorder with setRecorder if the key is exposed'; after a renounce neither is possible.

If beneficiary is unset at that moment, fund reverts NotConfigured('setBeneficiary') forever. A set recorder keeps recordJob/recordJobs/voidJob forever. The author's test test_renounceFreezesConfigurationButNotFunding shows the behaviour is known, but the README promise is wrong.

Minimal fix preserving the design: override renounceOwnership to revert (or require a prior explicit arming step), and list it in the README's access table. Reproduced in test/scratch/Review.t.sol::test_renounceStrands.

Merged from audit_permissions, audit_flow and audit_economics. Application-contract constructorArgs resolve only static words, $token, $requester and $contract:EarlierName; $owner is not one of them.

Unlike the specialists' reports, a launch.json now exists in the tree and its OffsetsRegistry entry is ["$requester", "$token"], which resolves to economics.remainderTo (0x556bbff02ea3b61431e2d80e4845a90db4526352) and the token; the registry constructor accepts it (non-zero owner, non-zero token) and no deployment is blocked.

What remains is a documentation inconsistency: README.md line 85 and the sentence 'The manifest step writes launch.json; this repository does not' (line 94) no longer describe the tree.

Fix: state $requester (or a static lowercase address) in the table and note that the owner is the requester's remainder address, which then controls beneficiary, recorder, rates and the seat NFT.