КОД-РЕВЬЮ СмартКонтракта Solidity. Контракт переводит деньги на 2 кошелька. Всё правильно написал?

Question

[solidussnake]

Я написал смарт контракт на Солидити для Эфириум и Полигон. Контракт должен получать деньги на свой адрес и автоматически 50% переводить на первый адрес кошелька, 25% переводить на второй кошелек. Прошу Вас сделать код-ревью моего контракта и указать на мои ошибки и как их можно исправить. Меня особенно интересует насколько этот контракт безопасный.

P.S. Спасибо, что прочитали.

Smart Contract:

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

contract Demo {
        constructor() payable{}
        receive() external payable{}

        uint _50 = address(this).balance/100*50; //50% of balance of contract
        uint _25 = address(this).balance/100*25; //25% of balance of contract

        address payable addr1 = address(0x583031D1113aD414F02576BD6afaBfb302140225);
        address payable addr2 = address(0xdD870fA1b7C4700F2BD7f44238821C26f7392148);

        function withdrawFunds() external {
            //address target = payable(_to);
            addr1.transfer(_50);
            addr2.transfer(_25);
        }

        function receiveFunds() external payable {}

        function getBalance() public view returns(uint){
            return address(this).balance;
        }
}

Answer 1

[Victoria Zotova]

Вот эти две сточки

uint _50 = address(this).balance/100*50; //50% of balance of contract
uint _25 = address(this).balance/100*25; //25% of balance of contract

будут выполняться во время работы конструктора. Т.е. если заранее на адрес контракта (до его создания) не отослать ничего - то address(this).balance будет равен 0. Эти переменные надо вычислять каждый раз, когда нужно переводить eth.
Также рекомендую сначала делать все умножение, а потом все деление: меньше проблем с округлением.
Чтобы контракт срабатывал автоматически, метод перевода средств должен быть в receive() external payable{}. И лучше не использовать balance, т.к. в него будут попадать прошлые остатки. В этом плане подойдет msg.value (если говорить об автоматическом переводе).
address payable addr1 и вторую переменную стоит сделать константой, если не планируется обновлять эти адреса в будущем. А если говорим о промышленном коде с тестированием, то неплохой вариант сделать их immutable и задавать в конструкторе.
Помимо этого необходим метод для вывода оставшихся 25% иначе застрянут на адресе контракта на совсем.
И наверное добавлю еще одну мелочь: transfer не самый идеальный способ переводить средства (если говорим о реальном контракте), из-за ограничения по газу на такой методу, рекомендую глянуть Address.sol (кажется метод sendValue) из библиотеки openzeppelin.

Comments

[konstantinst13]

Спасибо! :)

[konstantinst13]

Я применил ваши поправки.
Только почему-то метод receive() external payable{this.withdrawFunds();} не срабатывает автоматически.
Переписанный контракт в этом моём новом вопросе: https://qna.habr.com/q/1132696

[Victoria Zotova]

receive срабатывает, если просто послать средства на контракт без указания метода/функции.
Как именно выглядит вызов метода?