Instrumentation for `immutable`

[kasperpawlowski]

If state variable annotation present on a contract, scribble generates an auxiliary function with the original constructor body and calls it in the the constructor. That newly generated function is preceded/succeeded by scribble generated code, according to the annotations. However, if an immutable is set in the constructor, the newly generated function tries to set the immutable inside its body which is not allowed by solidity. When the contract is being compiled after arming, the following error can be observed:

[⠒] Compiling...
[⠑] Compiling 1 files with 0.8.20
[⠘] Solc 0.8.20 finished in 9.39ms
Error: 
Compiler run failed:
Error (1581): Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor.
  --> src/Example.sol:21:9:
   |
21 |         x = 5;
   |         ^

Example contract:

/// #if_succeeds x == 5;
contract Example {
    uint private immutable x;
    constructor() {
        x = 5;
    }
}

After arming:

/// #if_succeeds x == 5;
contract Example {
    uint private immutable x;

    constructor() {
        _original_Example_constructor();
        unchecked {
            if (!(x == 5)) {
                emit __ScribbleUtilsLib__14.AssertionFailed("000389:0066:000 0: ");
                assert(false);
            }
        }
    }

    function _original_Example_constructor() private {
        x = 5;
    }
}

library __ScribbleUtilsLib__14 {
    event AssertionFailed(string message);

    event AssertionFailedData(int eventId, bytes encodingData);

    function assertionFailed(string memory arg_0) internal {
        emit AssertionFailed(arg_0);
    }

    function assertionFailedData(int arg_0, bytes memory arg_1) internal {
        emit AssertionFailedData(arg_0, arg_1);
    }

    function isInContract() internal returns (bool res) {
        assembly {
            res := sload(0x5f0b92cf9616afdee4f4136f66393f1343b027f01be893fa569eb2e2b667a40c)
        }
    }

    function setInContract(bool v) internal {
        assembly {
            sstore(0x5f0b92cf9616afdee4f4136f66393f1343b027f01be893fa569eb2e2b667a40c, v)
        }
    }
}

Scribble version used: 0.6.23

[blitz-1306]

Hello there. I took some time to look into it and figure out what is happening here. Here are few clues:

1. For each of the contracts, Scribble finds annotations, that are attached in their docstring. Particularly interesting is following:

   scribble/src/bin/scribble.ts

   Lines 370 to 377 in ccc74a7

   	const allProperties = gatherContractAnnotations(contract, annotMap);
   	const allowedFuncProp = allProperties.filter(
   	(annot) =>
   	(annot instanceof PropertyMetaData &&
   	(annot.parsedAnnot.type === AnnotationType.IfSucceeds ||
   	annot.parsedAnnot.type === AnnotationType.Require)) ||
   	annot instanceof TryAnnotationMetaData
   	);

2. There is an interesting comment here, that is stating that we should avoid instrumenting functions for a set circumstances:

   scribble/src/bin/scribble.ts

   Lines 395 to 411 in ccc74a7

   	/**
   	* We interpose on functions if either of these is true
   	* a) They have annotations
   	* b) They are external or public AND they modify state (not constant/pure/view) AND they are not the constructor AND they are not fallback/receive
   	*
   	* Note: Constructors are instrumented in instrumentContract, not by instrumentFunction. fallback() and receive() don't check state invariants.
   	*/
   	if (
   	annotations.length > 0 ||
   	(needsStateInvariantInstr &&
   	isExternallyVisible(fun) &&
   	isChangingState(fun) &&
   	contract.kind === ContractKind.Contract &&
   	fun.kind === FunctionKind.Function)
   	) {
   	worklist.push([fun, annotations]);
   	}

   Still, this check only happends when we check for annotations, that are attached to contracts.
3. Some lines below, we have following section:

   scribble/src/bin/scribble.ts

   Lines 436 to 441 in ccc74a7

   	const contract = target.vScope;
   	assert(
   	contract instanceof ContractDefinition,
   	`Function instrumentation allowed only on contract funs`
   	);
   	instrumentFunction(ctx, annotations, target, contractsNeedingInstr.has(contract));

   It is calling an instrumentFunction, that is causing function to be interposed.

I guess allowedFuncProp have an entry, that forces constructor to be processed by instrumentFunction. From the inline comments I'm not sure if it is intended behavior.

I'm currently consider options to introduce a fix.

[blitz-1306]

Ofc, that seems to be a bug, as instrumented code can not be compiled.

[blitz-1306]

I also think, that issue can be related to https://docs.scribble.codes/language/annotations/lang-function-specifications#contract-level-if_succeeds-annotations - if_succeeds somehow causing constructor to be instrumented, however inline docs state that it should not happen. Im still investigating and gathering more clues to consider a proper fix.

Easy (and probably not the best) solution would be to process all of the state variables and make them mutable. However, I believe there should be a better solution.

[blitz-1306]

While we are finishing discussion and review for the fix, anyone who hits the issue could use tweaked Scribble from branch rewrite-immutable-state-vars of PR #239.

Steps to manually install Scribble from the repository or specific branch are described in README. Just checkout specific branch before installing.

[kasperpawlowski]

thanks for prompt resolution!

[blitz-1306]

@kasperpawlowski Hey there. New Scribble release is out. It should no longer have an issue when installed from NPM. Give it a try. Thanks for the feedback.