지금까지 작성했던 3개의 컨트랙트를 가지고, 사내 오디터분들과 같이 solidity답게 코드가 작성 됐는지 리뷰를 진행했습니다. 입사 후 진행하는 첫 코드 리뷰이기도 하고, 처음으로 스스로 작성해 본 컨트랙트라서 그런지 더 기대가 되었습니다 😙

오디터(Auditor)란? 

사전적으로는 '어떤 일을 검사하는 사람'이라는 뜻인데요. 수호의 오디터는 고객사의 컨트랙트가 정상적으로 작동하는지 분석하고, 해킹이 일어날 수 있는 취약점을 찾아내는 직무를 의미해요. 수호에서는 Audit 스쿼드의 Hackability와 Jasper가 오디터로서 안전한 블록체인 생태계를 만드는 데 기여하고 있어요!

또한 Solidity를 이제 막 배우기 시작하는 단계에서 이렇게 뛰어난 실력자분들께 코드 리뷰를 받을 수 있다는 것 자체만으로도 너무 좋은 기회였습니다. 이번 포스트에서는 어떤 코드 리뷰들이 오고 갔는지 공유해 보고자 합니다.

동일한 Storage Call 반복은 지양할 것

storage에 저장된 변수를 가져오는 SLOAD의 가스 소모량이 매우 크기 때문에, 불필요하게 SLOAD를 반복하는 것은 지양해야 합니다.

이더리움 웹페이지에서 Solidity의 각 연산이 EVM에서 얼만큼의 가스를 사용하는지 정리해둔 표를 확인할 수 있는데요.

SLOAD는 cold access인 경우 2100, warm access인 경우 100만큼의 가스를 소모합니다. (cold access는 최초 접근하는 경우, warm access는 두 번째 이후 접근하는 경우로 생각하면 됩니다.) 참조

반면 메모리에서 값을 읽어오는 MLOAD는 3만큼의 가스만 사용합니다. 이처럼 MLOAD가 SLOAD보다 훨씬 저렴하기 때문에, SLOAD는 최대한 지양하고 MLOAD로 최대한 대체할 수 있도록 코드를 작성해야 합니다.

쉬운 이해를 위해 단순한 예시를 들어보겠습니다. 참조

contract Test {
    uint one = 1;
    function test() returns (uint){
        return one + one;
    }
}

Remix에서 위 코드로 디버깅을 진행해보면, SLOAD 가 두 번 호출되는 것을 확인할 수 있습니다. Solidity에서 storage 변수를 읽어올 때 캐싱을 진행하지 않기 때문인데요.

이것 때문에 제가 CafeMenu 컨트랙트에서 작성한 아래 코드는 많은 양의 가스를 소모하게 됩니다.

// at function createMaterial()
// 새로운 재료 이름의 중복 여부 체크
for(uint i = 0; i < materials.length; i++){
    require(
        !equals(materials[i].name, _name), 
        string(abi.encodePacked("Material ", _name, " already exists."))
    );
}

for문에서 루프가 반복될 때마다 매번 storage의 materials.length를 참조하기 때문에, SLOAD가 불필요하게 여러 번 반복 호출되어 가스를 낭비하게 되는 것입니다. 그러면 가스를 절약하기 위해서는 어떻게 해야 할까요?

uint materialCount = materials.length;
for(uint i = 0; i < materialCount; i++){
    require(
        !equals(materials[i].name, _name), 
        string(abi.encodePacked("Material ", _name, " already exists."))
    );
}

저는 materials.length를 새로운 변수로 선언한 후, 이를 for문 내에서 사용하였습니다. 이렇게 되면 SLOAD는 변수를 최초에 선언할 때만 호출이 되고, 그 이후는 메모리를 참조하는 MLOAD를 호출하기 때문에 가스를 훨씬 아낄 수 있습니다.

function createProduct(string memory _name, Recipe[] calldata _recipes)
		public
		onlyOwner
		returns(uint) 
{
		uint countRecipes = _recipes.length;
		...
}

반면 위와 같이 calldata변수를 참조하는 경우는 CALLDATALOAD를 호출하는데, 여기에도 3만큼의 가스비가 들기 때문에 위 코드처럼 메모리에 새로운 변수를 할당하는 것이 오히려 가스를 더 소모하는 것이라고 합니다.

for문보다는 mapping?

같이 코드 리뷰를 진행해 주신 Hackability께서 가스를 많이 소모하는 지점으로 지적해 주신 곳은 두 곳이었습니다.

첫 번째, 사용자에게 새로운 재료 토큰을 발급해줄 때

제가 작성했던 컨트랙트의 프로세스는 다음과 같습니다. (MaterialFactory 컨트랙트의 issuMaterialToken 함수를 참고해주세요.)

  1. 사용자가 가지고 있는 토큰들의 목록을 mapping(address => MaterialToken[]) public ownerToTokens에서 찾습니다.
  2. 새로 발급해 줄 토큰을 사용자가 이미 가지고 있는지 찾습니다. 이미 갖고 있다면 사용자가 갖고 있는 MaterialToken에서 defaultAmount만큼 더해주고, 갖고 있지 않다면 새로운 MaterialToken을 만들어 ownerToTokens 배열에 push해 줍니다.

최악의 경우, 사용자가 m가지 종류의 재료를 가지고 있다면 최대 m번만큼의 탐색을 진행해야 하는 것이죠.

두 번째, 제품 토큰으로 교환하기 위해 재료 토큰들을 소진할 때

(ProductFactory 컨트랙트의 _payMaterials 함수를 참고해주세요.)

이 함수는 더 복잡하기 때문에 예시를 들어보겠습니다:)

사용자가 5번 토큰을 150, 4번 토큰을 20, 3번 토큰을 200, 7번 토큰을 10만큼 가지고 있었다고 가정하고, 이를 [(5, 150), (4, 20), (3, 200), (7, 10)]으로 표현해 보겠습니다.

그리고 사용자가 교환하고자 하는 제품을 만들기 위해서는 3번 토큰이 150, 4번 토큰이 10만큼 필요하다고 가정해 보겠습니다.

아래는 사용자들이 재료 토큰들을 충분히 가지고 있는지 체크하는 부분인데요, 이중 for문을 보니 벌써부터 어지러워집니다...😵‍💫

MaterialToken[] memory tokens = ownerToTokens[msg.sender];

// 각 material들이 payable한지 체크
for(uint p = 0; p < _recipes.length; p++){
    bool hasTargetMaterial = false;

    for(uint i = 0; i < tokens.length; i++){
        if( tokens[i].materialId != _recipes[p].materialId )
            continue;

        hasTargetMaterial = true;
        uint balance = tokens[i].amount;
        require(
            balance >= _recipes[p].amount, 
            string(abi.encodePacked("Your balance of material ", _recipes[p].materialId, "is insufficient"))
        );

        ownerTargetMaterialTokenIds[p] = i;
        break;
    }

    // 타겟 material을 가지고 있고, 위의 require문들을 통과해야(가진 재료 양이 기준보다 많아야)한다.
    require(
        hasTargetMaterial, 
        string(abi.encodePacked("You don't have a material (ID:", _recipes[p].materialId, ")"))
    );
}

바깥쪽 for문의 첫 번째 루프에서는 레시피의 첫 번째 재료가 충분히 있는지 체크합니다. 저희 예시에서는 3번 토큰이 150 이상 있는지 확인하면 되겠죠!

이제 사용자가 가지고 있는 토큰 배열(tokens)을 하나하나 확인해 보아야 합니다. tokens의 첫 번째 요소는 5번 재료이니 continue, 그다음 요소는 4번 재료이니 continue, 그다음 요소가 3번 재료이고, 양도 150 이상 가지고 있으므로 for문을 빠져나옵니다.

이런 식으로 레시피에 필요한 재료의 개수만큼 반복을 진행하게 되는데요.

사용자가 m가지 종류의 재료를 가지고 있고 레시피에 필요한 재료 종류가 n개라면 for문을 순회하는 동안 최악의 경우 SLOAD를 mn번 발생시키기 때문에 가스를 너무 많이 낭비하게 됩니다.

그래서, 자료 구조를 mapping(address => MaterialToken[]) public ownerToTokens에서

mapping(address => mapping (uint => uint)) public materialTokenBalanceOf으로 변경한 것입니다.

가지고 있는 재료 토큰들을 배열로 저장하는 것이 아니라, mapping (uint => uint) 의 구조, 즉 (materialId ⇒ amount)의 형태로 저장하도록 한 거죠.

이렇게 변경하고 나니, 첫 번째 문제는 너무 쉽게 해결되었습니다.

function issueMaterialToken(uint _materialId, address _to) external onlyOwner {
    (,, uint32 defaultAmount) = cafeMenu.materials(_materialId);

    materialTokenBalanceOf[_to][_materialId] += defaultAmount;
}

for문을 돌지 않고도, 코드 단 한 줄에 토큰을 defaultAmount만큼 추가할 수 있게 된 것입니다!

하지만 한 가지 문제가 생겼는데요, 사용자가 가지고 있는 모든 토큰의 양을 조회하는 materialTokensOf 함수였습니다. 기존에는 ownerToTokens[msg.sender] 로 단 한 줄 만에 끝낼 수 있는 함수였는데, 이제는 사용자가 어떤 재료를 가지고 있는지, 즉 mapping의 key 목록을 알 수 없게 되어 문제가 어려워진 것이었습니다.

자바스크립트의 Object.keys나 Object.entries와 같은 함수가 있었다면 좋았을 텐데... Solidity에서는 기대도 하면 안 되는 고급 함수기 때문에... 다른 방법을 생각해야만 했습니다.

어차피 external view 함수인데 그냥 0부터 material.length까지 다 넣어볼까 싶기도 했었지만, 아래와 같이 사용자가 가지고 있는 토큰의 Id들을 저장하는, 즉 mapping의 key 목록을 저장해두는 배열을 사용하기로 했습니다.

mapping(address => uint[]) private _materialTokenIdsOf;

그리고 토큰을 발급해줄 때 기존에 갖고 있던 양이 0이었으면 배열에 새로 key를 추가해주고, 해당 재료를 제품으로 교환하여 모두 소진했으면 배열에서 삭제함으로써 key 배열을 유지하기로 했습니다.

function _addOwnTokenId(address _owner, uint _materialId) internal {
    _materialTokenIdsOf[_owner].push(_materialId);
}

// 재료 소진 시 token balance가 0이 되는 경우에 호출
function _removeOwnTokenId(address _owner, uint _materialId) internal {
    int index = -1;
    uint[] storage ownerTokenIds = _materialTokenIdsOf[_owner];
    uint ownedTokenCount = ownerTokenIds.length;

    require(ownedTokenCount > 0, "You have no materials");

    for(uint i = 0; i < ownedTokenCount; i++){
        if(ownerTokenIds[i] == _materialId){
            index = int(i);
            break;
        }
    }
    require(index != -1, "Material not found");

    ownerTokenIds[uint(index)] = ownerTokenIds[ownedTokenCount-1];
    delete ownerTokenIds[ownedTokenCount-1];
}

key를 추가하는 경우는 단순히 배열에 push 하기만 하면 됩니다.

삭제하는 경우는

  1. 삭제해야 하는 요소의 index를 찾고,
  2. 배열의 마지막 요소를 해당 index 자리에 넣어두고,
  3. 배열의 마지막 요소를 삭제합니다.

key 목록을 유지하는 배열을 만들었으니, 아까 재료를 발급해주는 issueMaterialToken 함수에 _addOwnTokenId 를 추가해 줍니다.

// add this
if(materialTokenBalanceOf[_to][_materialId] == 0){
    _addOwnTokenId(_to, _materialId);
}

materialTokenBalanceOf[_to][_materialId] += defaultAmount;

두 번째 문제도 훨씬 쉽게 해결되었습니다!

function _payMaterials(CafeMenu.Recipe[] memory _recipes) internal {
    mapping (uint => uint) storage ownedMaterialTokens = materialTokenBalanceOf[msg.sender];
    
    for (uint i = 0; i < _recipes.length; i++) {
        require(
            ownedMaterialTokens[_recipes[i].materialId] >= _recipes[i].amount, 
            string(abi.encodePacked("Your balance of material ", _recipes[i].materialId, "is insufficient"))
        );

        // material들이 payable하면 pay
        ownedMaterialTokens[_recipes[i].materialId] -= _recipes[i].amount;
        if(ownedMaterialTokens[_recipes[i].materialId] == 0){
            // 모든 재료를 소진했으면 가지고 있는 토큰 ID 목록에서 제거
            _removeOwnTokenId(msg.sender, _recipes[i].materialId);
        }
    }
}

일단 언뜻 보기에도 코드 길이부터 확 줄었죠! 42줄에서 21줄로, 정확히 1/2로 줄었습니다!

mapping에서 사용자가 가지고 있는 재료 토큰의 양을 바로 확인할 수 있어 이중 반복 없이 빠르게 작업을 수행할 수 있었습니다!

또 한 가지, 함수 내에서 require 등으로 인해 revert 되는 경우, 이전에 실행된 storage의 변경사항은 처음으로 되돌려집니다.

기존에는 이걸 몰라 모든 재료의 양이 충분한지 확인된 이후에야 재료들을 소비했는데, 이제는 마음 편하게 각 재료별로 양이 충분한지 확인 후 재료를 소진하도록 바꿀 수 있었습니다!

코드 길이도 줄고, 가독성도 훨씬 좋아졌고, 무엇보다 기존에 mn번 + n번의 SLOAD가 필요하던 함수를 단 n번의 호출만으로 끝낼 수 있었습니다😜

제가 작성하는 컨트랙트 코드가 가스 소모량과 직결되고, 생각하지 못했던 보안 취약점이 언제 어디서 나타날지 모른다는 것이 컨트랙트 개발을 가장 어렵게 느끼도록 하는데요. 반대로 그런 점이 컨트랙트 개발을 더 흥미롭게 해주는 게 아닐까 생각합니다. 무엇보다 옆에 든든한 오디터 분들이 계시니 자신감이 더 붙어서 실력도 더 빨리 성장하는 것 같아요!

Cafe NFT Project 시리즈 모아보기
수호 카페에 NFT가 생긴다면?
컨트랙트 개발 시작 - ERC721Enumerable
컨트랙트 코드 리뷰 - 가스비 절약하기
스마트 컨트랙트와 상호작용하기