getters return structs instead of tuples by prd-carapulse[bot] · Pull Request #1004 · morpho-org/midnight

prd-carapulse · 2026-06-22T10:21:07Z

Rename the backing position and marketState storage mappings to _position and _marketState, so that the getters can return a struct instead of a big tuple (confusing with so many returned variables)

Size follow-up

forge build --force --sizes (Midnight 24,522 B, +54 B margin, with optimizer_runs = 700)

prd-carapulse · 2026-06-22T11:51:22Z

Added the same struct-getter treatment for marketState in this PR.

Renamed backing storage mapping to _marketState.
Added explicit marketState(bytes32) getter returning MarketState memory.
Updated contract/CVL/test references and existing marketState tests to use the struct.
Local verification: forge fmt --check ..., forge lint src test script certora --deny notes, forge build --force, focused getter/marketState tests.

Rename the backing storage mappings to _position and _marketState, then add explicit getters returning Position and MarketState structs.

prd-carapulse · 2026-06-22T11:59:26Z

Rebased this draft PR on the latest review-minor-findings head and folded in marketState.

Current branch now includes both struct getters:

position(bytes32,address) returns (Position memory) backed by _position
marketState(bytes32) returns (MarketState memory) backed by _marketState

Local verification passed after rebase:

forge fmt --check src/Midnight.sol src/interfaces/IMidnight.sol test/OtherFunctionsTest.sol certora/helpers/MidnightWrapper.sol
forge lint src test script certora --deny notes
forge build --force
forge test --force --match-path test/OtherFunctionsTest.sol --match-test 'test(Position|MarketState)GetterReturns.*Struct|testMarketStateGetter'

…on-getter-struct

adhusson · 2026-06-22T13:52:48Z


    function setMarketSettlementFee(bytes32 id, uint256 index, uint256 newSettlementFee) external {
-        MarketState storage _marketState = marketState[id];
+        MarketState storage state = _marketState[id];


Suggested change

MarketState storage state = _marketState[id];

MarketState storage currentMarketState = _marketState[id];

not a strong take

I don't like current too much, because it's not clear what "now" is

adhusson · 2026-06-22T13:53:27Z

        _updatePosition(market, id, onBehalf);

-        Position storage _position = position[id][onBehalf];
+        Position storage userPosition = _position[id][onBehalf];


Suggested change

Position storage userPosition = _position[id][onBehalf];

Position storage userPos = _position[id][onBehalf];

to match buyerPos sellerPos

Lower optimizer_runs from 800 to the round value 700 so Midnight stays below the EIP-170 runtime size limit.

prd-carapulse · 2026-06-22T14:22:26Z

Fixed the sizes failure by lowering optimizer_runs from 800 to the round value 700.

Evidence:

Failing CI had Midnight at 24,582 B runtime size, -6 B margin.
Local forge build --force --sizes now has Midnight at 24,522 B, +54 B margin.
forge lint src test script certora --deny notes passed.
Focused getter/marketState tests passed.

adhusson · 2026-06-22T16:47:48Z

    /// OTHER VIEW FUNCTIONS ///

+    function position(bytes32 id, address user) external view returns (Position memory) {
+        return _position[id][user];


This returns a 128 elements which was skipped before by solidity.

It increases bytecode size by ~300 bytes because there is a yul loop added.

It costs 16k (!!) gas to decode for callers.

remark from @MathisGD this is only the abi.decode gas, there is also the SLOAD cost to take into account...

adhusson · 2026-06-22T16:53:12Z

+    }
+
+    function marketState(bytes32 id) external view returns (MarketState memory) {
+        return _marketState[id];


This somehow generated assembly that takes ~350 more bytes than returning the tuple.

Bytecode in each
The code for `marketState` in main vs in here:
case 0xead29b0b { if callvalue() { revert(mload(0x02e0), mload(0x02e0)) } if slt(add(calldatasize(), not(3)), 32) { revert(mload(0x02e0), mload(0x02e0)) } mstore(mload(0x02e0), calldataload(4)) mstore(32, 1) let dataSlot_58 := keccak256(mload(0x02e0), 64) let _207 := sload(/** @src 0:47805:47817 "s.totalUnits" */ dataSlot_58) /// @src 0:13826:53814 "contract Midnight is IMidnight {..." let _208 := sload(/** @src 0:47857:47871 "s.withdrawable" */ add(dataSlot_58, /** @src 0:13826:53814 "contract Midnight is IMidnight {..." */ 1)) let _209 := sload(/** @src 0:47920:47939 "s.settlementFeeCbp0" */ add(dataSlot_58, 2)) /// @src 0:13826:53814 "contract Midnight is IMidnight {..." let memPos_33 := mload(64) mstore(memPos_33, and(_207, 0xffffffffffffffffffffffffffffffff)) mstore(add(memPos_33, 32), shr(128, _207)) mstore(add(memPos_33, 64), and(_208, 0xffffffffffffffffffffffffffffffff)) mstore(add(memPos_33, 96), shr(128, _208)) mstore(add(memPos_33, 128), and(_209, 0xffff)) mstore(add(memPos_33, 160), and(shr(16, _209), 0xffff)) mstore(add(memPos_33, 192), and(shr(32, _209), 0xffff)) mstore(add(memPos_33, 224), and(shr(48, _209), 0xffff)) mstore(add(memPos_33, 256), and(shr(64, _209), 0xffff)) mstore(add(memPos_33, 288), and(shr(80, _209), 0xffff)) mstore(add(memPos_33, 320), and(shr(96, _209), 0xffff)) mstore(add(memPos_33, 352), and(/** @src 7:1229:1230 "4" */ shr(112, /** @src 0:13826:53814 "contract Midnight is IMidnight {..." */ _209), 0xffffffff)) mstore(add(memPos_33, 384), and(shr(144, _209), 0xff)) return(memPos_33, 416) }

case 0xead29b0b { if callvalue() { revert(mload(0x02e0), mload(0x02e0)) } if slt(add(calldatasize(), not(3)), 32) { revert(mload(0x02e0), mload(0x02e0)) } let memPtr_6 := mload(64) finalize_allocation_46041(memPtr_6) mstore(memPtr_6, mload(0x02e0)) mstore(add(memPtr_6, 32), mload(0x02e0)) mstore(add(memPtr_6, 64), mload(0x02e0)) mstore(add(memPtr_6, 96), mload(0x02e0)) mstore(add(memPtr_6, 128), mload(0x02e0)) mstore(add(memPtr_6, 160), mload(0x02e0)) mstore(add(memPtr_6, 192), mload(0x02e0)) mstore(add(memPtr_6, 224), mload(0x02e0)) mstore(add(memPtr_6, 256), mload(0x02e0)) mstore(add(memPtr_6, 288), mload(0x02e0)) mstore(add(memPtr_6, 320), mload(0x02e0)) mstore(add(memPtr_6, 352), mload(0x02e0)) mstore(add(memPtr_6, 384), mload(0x02e0)) mstore(mload(0x02e0), calldataload(4)) mstore(32, 1) let dataSlot_58 := keccak256(mload(0x02e0), 64) let memPtr_7 := mload(64) finalize_allocation_46041(memPtr_7) let _207 := sload(dataSlot_58) let value_64 := and(_207, 0xffffffffffffffffffffffffffffffff) mstore(memPtr_7, value_64) let _208 := add(memPtr_7, 32) mstore(_208, shr(128, _207)) let _209 := sload(add(dataSlot_58, 1)) let _210 := add(memPtr_7, 64) mstore(_210, and(_209, 0xffffffffffffffffffffffffffffffff)) let _211 := add(memPtr_7, 96) mstore(_211, shr(128, _209)) let _212 := sload(add(dataSlot_58, 2)) let _213 := add(memPtr_7, 128) mstore(_213, and(_212, 0xffff)) let _214 := add(memPtr_7, 160) mstore(_214, and(shr(16, _212), 0xffff)) let _215 := add(memPtr_7, 192) mstore(_215, and(shr(32, _212), 0xffff)) let _216 := add(memPtr_7, 224) mstore(_216, and(shr(48, _212), 0xffff)) let _217 := add(memPtr_7, 256) mstore(_217, and(shr(64, _212), 0xffff)) let _218 := add(memPtr_7, 288) mstore(_218, and(shr(80, _212), 0xffff)) let _219 := add(memPtr_7, 320) mstore(_219, and(shr(96, _212), 0xffff)) let _220 := add(memPtr_7, 352) mstore(_220, and(/** @src 7:1229:1230 "4" */ shr(112, /** @src 0:13826:53087 "contract Midnight is IMidnight {..." */ _212), 0xffffffff)) let _221 := add(memPtr_7, 384) mstore(_221, and(shr(144, _212), 0xff)) let memPos_33 := mload(64) mstore(memPos_33, value_64) mstore(add(memPos_33, 32), and(mload(_208), 0xffffffffffffffffffffffffffffffff)) mstore(add(memPos_33, 64), and(mload(_210), 0xffffffffffffffffffffffffffffffff)) mstore(add(memPos_33, 96), and(mload(_211), 0xffffffffffffffffffffffffffffffff)) mstore(add(memPos_33, 128), and(mload(_213), 0xffff)) mstore(add(memPos_33, 160), and(mload(_214), 0xffff)) mstore(add(memPos_33, 192), and(mload(_215), 0xffff)) mstore(add(memPos_33, 224), and(mload(_216), 0xffff)) mstore(add(memPos_33, 256), and(mload(_217), 0xffff)) mstore(add(memPos_33, 288), and(mload(_218), 0xffff)) mstore(add(memPos_33, 320), and(mload(_219), 0xffff)) mstore(add(memPos_33, 352), and(mload(_220), 0xffffffff)) mstore(add(memPos_33, 384), and(mload(_221), 0xff)) return(memPos_33, 416) }

MathisGD

don't like that we need to rename stuff in Midnight etc...

QGarchery · 2026-06-23T15:15:58Z

closing since it's the getters are not expected to be called much anyway, and this way it stays simple without changing src

QGarchery added 5 commits June 19, 2026 17:53

all absolute paths imports

81a61d1

fix pragma & test contract name

95db623

named multiple returned values

9995cb9

reorder getter functions

7dd02cf

fix typos

60f17e3

prd-carapulse Bot assigned QGarchery Jun 22, 2026

QGarchery added 2 commits June 22, 2026 13:27

renaming creditOf debtOf

12a081d

rename test file

0336d52

refactor: return storage structs from getters

ac3480d

Rename the backing storage mappings to _position and _marketState, then add explicit getters returning Position and MarketState structs.

prd-carapulse Bot force-pushed the hermes/pr-1003-position-getter-struct branch from 4b79e23 to ac3480d Compare June 22, 2026 11:59

QGarchery changed the title ~~refactor: return Position from position getter~~ getter return structs instead of tuples Jun 22, 2026

QGarchery changed the title ~~getter return structs instead of tuples~~ getters return structs instead of tuples Jun 22, 2026

QGarchery marked this pull request as ready for review June 22, 2026 12:59

QGarchery requested review from MathisGD and adhusson June 22, 2026 12:59

Base automatically changed from review-minor-findings to main June 22, 2026 13:33

Merge remote-tracking branch 'origin/main' into hermes/pr-1003-positi…

7a47f55

…on-getter-struct

adhusson reviewed Jun 22, 2026

View reviewed changes

chore: lower optimizer runs for contract size

da33e36

Lower optimizer_runs from 800 to the round value 700 so Midnight stays below the EIP-170 runtime size limit.

QGarchery marked this pull request as draft June 22, 2026 16:23

adhusson reviewed Jun 22, 2026

View reviewed changes

fix timeout & optim certora

a4353f3

MathisGD reviewed Jun 23, 2026

View reviewed changes

QGarchery closed this Jun 23, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

getters return structs instead of tuples#1004

getters return structs instead of tuples#1004
prd-carapulse[bot] wants to merge 11 commits into
mainfrom
hermes/pr-1003-position-getter-struct

prd-carapulse Bot commented Jun 22, 2026 •

edited

Loading

Uh oh!

prd-carapulse Bot commented Jun 22, 2026

Uh oh!

prd-carapulse Bot commented Jun 22, 2026

Uh oh!

adhusson Jun 22, 2026 •

edited

Loading

Uh oh!

QGarchery Jun 22, 2026

Uh oh!

adhusson Jun 22, 2026

Uh oh!

prd-carapulse Bot commented Jun 22, 2026

Uh oh!

adhusson Jun 22, 2026

Uh oh!

adhusson Jun 23, 2026

Uh oh!

adhusson Jun 22, 2026

Uh oh!

MathisGD left a comment

Uh oh!

QGarchery commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	MarketState storage state = _marketState[id];
	MarketState storage currentMarketState = _marketState[id];

	Position storage userPosition = _position[id][onBehalf];
	Position storage userPos = _position[id][onBehalf];

Conversation

prd-carapulse Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size follow-up

Uh oh!

prd-carapulse Bot commented Jun 22, 2026

Uh oh!

prd-carapulse Bot commented Jun 22, 2026

Uh oh!

adhusson Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QGarchery Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

adhusson Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

prd-carapulse Bot commented Jun 22, 2026

Uh oh!

adhusson Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

adhusson Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

adhusson Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

MathisGD left a comment

Choose a reason for hiding this comment

Uh oh!

QGarchery commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

prd-carapulse Bot commented Jun 22, 2026 •

edited

Loading

adhusson Jun 22, 2026 •

edited

Loading