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

🐛 Fix logMemory in safeconsole #591

Merged
merged 2 commits into from
Jul 31, 2024
Merged

🐛 Fix logMemory in safeconsole #591

merged 2 commits into from
Jul 31, 2024

Conversation

Philogy
Copy link
Contributor

@Philogy Philogy commented Jul 30, 2024

The safeconsole.logMemory method did not work as expected because it was calling the logBytes(bytes) method on the console pre-compile and not log(bytes).

I furthermore marked all assembly blocks as "memory safe" as it doesn't change memory and to prevent it from conflicting with certain viaIR optimizations.

@mds1
Copy link
Collaborator

mds1 commented Jul 30, 2024

Sorry where is log(bytes) defined? I do not see that method in the console libraries

@Philogy
Copy link
Contributor Author

Philogy commented Jul 30, 2024

Sorry where is log(bytes) defined? I do not see that method in the console libraries

@mds1

function logBytes(bytes memory p0) internal pure {
_sendLogPayload(abi.encodeWithSignature("log(bytes)", p0));
}

@mds1
Copy link
Collaborator

mds1 commented Jul 30, 2024

That signature you linked is logBytes(bytes) which is 0xe17bf956 (what was originally in console.sol), but the PR changes it log(bytes) (0x0be77f56) and I don't see a function log(bytes) defined anywhere

@Philogy
Copy link
Contributor Author

Philogy commented Jul 30, 2024

That signature you linked is logBytes(bytes) which is 0xe17bf956 (what was originally in console.sol), but the PR changes it log(bytes) (0x0be77f56) and I don't see a function log(bytes) defined anywhere

So the library function is called logBytes(bytes) (which is also why I initially put that) but it dispatches out (the actual call to the underlying "console pre-compile" address) log(bytes).

You can try it yourself locally by testing the code from my PR:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import {Test} from "../src/Test.sol";
import {safeconsole} from "../src/safeconsole.sol";

/// @author philogy <https://github.com/philogy>
contract safeconsoleTest is Test {
    struct A {
        uint256 a;
        uint256 b;
    }

    function test_logMemory() public pure {
        A memory a = A(uint256(keccak256("a")), uint256(keccak256("b")));
        safeconsole.logMemory(0x00, 0x100);
    }
}

Here also a link to foundry's actual definition of the underlying console interface: https://github.com/foundry-rs/foundry/blob/master/crates/evm/abi/src/HardhatConsole.json (you can use this query jq '[.[] | select(.inputs | map(.type) | any(. == "bytes"))]' HardhatConsole.json to find the log(bytes) method)

@mds1
Copy link
Collaborator

mds1 commented Jul 31, 2024

I see now, thank you! Not sure how I missed that when you linked to this 😅

function logBytes(bytes memory p0) internal pure {
_sendLogPayload(abi.encodeWithSignature("log(bytes)", p0));
}

@mds1 mds1 merged commit 4d63c97 into foundry-rs:master Jul 31, 2024
3 checks passed
@Philogy
Copy link
Contributor Author

Philogy commented Jul 31, 2024

No worries, happens, thanks for being so responsive!

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.

2 participants