[internal test - Contract Bugs]: [Suggest] guard upgrade

DevUreak commented 2 weeks ago

Describe the bug

Describe the bug and its behavior! You can reference the problematic code using links (internal-test-(2024.10) branch)

Storing storage uses high gas, Reentrancy-related locking functions perform unnecessary writes. This function is available from solidity 0.8.24, and it is easy to convert, so it would be good to consider.

Impact

The current version of the code is as follows :

    // Layer2ManagerStorage.sol
    modifier ifFree {
        require(!_lock, "lock");
        _lock = true;
        _;
        _lock = false;
    }

I've previously written code specs like this (It can be of similar code spec) :

modifier guard() {
 assembly ("memory-safe"){
      if tload(0){
         revert(0,0)  
     }
     tstore(0,1)
 }
 __;
 assembly ("memory-safe"){
    tstore(0,0)
 }
}

Exploit Scenario

No response

Recommendation

contracts/layer2/Layer2ManagerStorage.sol

Zena-park commented 2 weeks ago

Thank you. @DevUreak During the hardhat test, an error occurred when using tstore, and the test did not work properly. If the test is successful, please send me the code and I will check it.

https://github.com/tokamak-network/ton-staking-v2/blob/79f17dee8e7f0d628f6f4479db1c03a0930e6fb0/contracts/layer2/Layer2ManagerV1_1.sol#L249-L263

스크린샷 2024-10-15 오전 12 24 40

DevUreak commented 1 week ago

I will fix it and let you know. thanks

Zena-park commented 1 week ago

@DevUreak Thank you. The above tests were run on the test-#48 branch.

DevUreak commented 1 week ago

An error occurred during testing in the test-#48 branch. It worked normally in Remix, but there seems to be a problem in the hardhat environment. There is no problem with tload(), but there seems to be a problem with tstore. @Zena-park

Zena-park commented 1 week ago

It hasn't been tested well and we don't have experience using it on mainnet yet, so we won't implement it this time. Thank you. @DevUreak

tokamak-network / ton-staking-v2