Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark inline assembly as memory-safe #78

Merged
merged 1 commit into from
Jan 9, 2025
Merged

Conversation

davidlaprade
Copy link
Contributor

Fixes #52

The GovernorCountingFractional contract will be removed in a future release when we bump OZ to v5.1. But for the sake of users that can't upgrade to OZ v5.1, we're marking the use of assembly in that contract as memory-safe.

Per the solidity docs:

image

The only assembly we have is here:

    function _decodePackedVotes(bytes memory voteData)
        internal
        pure
        returns (
            uint128 againstVotes,
            uint128 forVotes,
            uint128 abstainVotes
        )
    {
        assembly {
            againstVotes := shr(128, mload(add(voteData, 0x20)))
            forVotes := and(_MASK_HALF_WORD_RIGHT, mload(add(voteData, 0x20)))
            abstainVotes := shr(128, mload(add(voteData, 0x40)))
        }
    }

This assembly doesn't write to any memory variables. Nor does it allocate any. Instead, it assigns to return variables which solidity has already assigned to stack slots. It only reads from the voteData memory param, and does so within its already-allocated bounds (the caller would have reverted if voteData.length != 48). It doesn't modify any scratch space or the free memory pointer.

@davidlaprade davidlaprade marked this pull request as ready for review January 8, 2025 20:25
Copy link

github-actions bot commented Jan 8, 2025

Coverage after merging memory-safety-fixes-#52 into master will be

91.67%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   FlexVotingClient.sol100%100%100%100%
   FractionalPool.sol79.63%70.59%73.33%82.89%107, 109, 111, 114, 190–191, 193–195, 198, 202, 238–239, 78, 80, 82–83, 85
   GovernorCountingFractional.sol94.59%76.92%100%98.04%184, 186, 188, 191

@davidlaprade davidlaprade merged commit e435476 into master Jan 9, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

perf: verify then mark assembly blocks as memory safe
2 participants