Waltonchain === Summary --- Launched in 2016, the Waltonchain (WTC) project is building an ecosystem that melds Blockchain, RFID technology, and IoT (Internet of Things) with complete data exchange and absolute information transparency based on the blockchain. The Walton team develops and produces Transaction ID-reading RFID chips, which can generate their own random ID hashes that are uploaded simultaneously to the blockchain via their RFID reader. The RFID technology can be used in various sectors requiring identification (credit cards, mobile payments, magnetic keys, etc.) This translates to enhanced operational efficiency, especially for supply chain use cases such high-end clothing identification, food & drug traceability, and logistics tracking. Waltonchain has been implementing its RFID and other technical applications in the clothing, food, collection, logistics, and other industries. The team mainly focuses on two aspects: data reliability and data value circulation. By seamlessly integrating Blockchain and FRID to ensure data reliability right from the source, Waltonchain brings outstanding effectiveness of data sharing by leveraging its cross-chain ecosystem. Scope of Work --- CertiK was chosen by WaltonChain to audit the design and implementation of golang version blockchain, which is a fork of `go-ethereum 1.7.1` and its cross chain smart contracts. To ensure comprehensive protection,the source code has been analyzed by the proprietary CertiK manually reviewed by our blockchain and smart contract experts and engineers. That end-to-end process ensures proof of stability as well as a hands-on, engineering-focused process to close potential loopholes and recommend design changes in accordance with the best practices in the space. \begin{small} \begin{longtable}[ht]{ p{.20\textwidth} p{.75\textwidth} } \hline & File Modified \\ \hline 1 & \textcolor[rgb]{0.2,0.2,0.2}{accounts/abi/bind/backends/simulated.go} \\ \hline 2 & \textcolor[rgb]{0.2,0.2,0.2}{accounts/keystore/key.go} \\ \hline 3 & \textcolor[rgb]{0.2,0.2,0.2}{build/ci.go} \\ \hline 4 & \textcolor[rgb]{0.2,0.2,0.2}{cmd/bootnode/main.go} \\ \hline 5 & \textcolor[rgb]{0.2,0.2,0.2}{cmd/geth/chaincmd.go} \\ \hline 6 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{cmd/geth/dao\textunderscore{}test.go}} \\ \hline 7 & \textcolor[rgb]{0.2,0.2,0.2}{cmd/geth/main.go} \\ \hline 8 & \textcolor[rgb]{0.2,0.2,0.2}{cmd/geth/misccmd.go} \\ \hline 9 & \textcolor[rgb]{0.2,0.2,0.2}{cmd/geth/usage.go} \\ \hline 10 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{cmd/puppeth/wizard\textunderscore{}genesis.go}} \\ \hline 11 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{cmd/puppeth/wizard\textunderscore{}node.go}} \\ \hline 12 & \textcolor[rgb]{0.2,0.2,0.2}{cmd/utils/flags.go} \\ \hline 13 & \textcolor[rgb]{0.2,0.2,0.2}{common/big.go} \\ \hline 14 & \textcolor[rgb]{0.2,0.2,0.2}{common/now.go} \\ \hline 15 & \textcolor[rgb]{0.2,0.2,0.2}{consensus/consensus.go} \\ \hline 16 & \textcolor[rgb]{0.2,0.2,0.2}{consensus/ethash/algorithm.go} \\ \hline 17 & \textcolor[rgb]{0.2,0.2,0.2}{consensus/ethash/consensus.go} \\ \hline 18 & \textcolor[rgb]{0.2,0.2,0.2}{consensus/ethash/ethash.go} \\ \hline 19 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{consensus/ethash/ethash\textunderscore{}test.go}} \\ \hline 20 & \textcolor[rgb]{0.2,0.2,0.2}{consensus/ethash/sealer.go} \\ \hline 21 & \textcolor[rgb]{0.2,0.2,0.2}{consensus/misc/dao.go} \\ \hline 22 & \textcolor[rgb]{0.2,0.2,0.2}{console/console.go} \\ \hline 23 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{core/block\textunderscore{}validator.go}} \\ \hline 24 & \textcolor[rgb]{0.2,0.2,0.2}{core/blockchain.go} \\ \hline 25 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{core/chain\textunderscore{}makers.go}} \\ \hline 26 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{core/dao\textunderscore{}test.go}} \\ \hline 27 & \textcolor[rgb]{0.2,0.2,0.2}{core/evm.go} \\ \hline 28 & \textcolor[rgb]{0.2,0.2,0.2}{core/genesis.go} \\ \hline 29 & \textcolor[rgb]{0.2,0.2,0.2}{core/headerchain.go} \\ \hline 30 & \textcolor[rgb]{0.2,0.2,0.2}{core/state/dump.go} \\ \hline 31 & \textcolor[rgb]{0.2,0.2,0.2}{core/state/journal.go} \\ \hline 32 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{core/state/state\textunderscore{}object.go}} \\ \hline 33 & \textcolor[rgb]{0.2,0.2,0.2}{core/state/statedb.go} \\ \hline 34 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{core/state\textunderscore{}processor.go}} \\ \hline 35 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{core/state\textunderscore{}transition.go}} \\ \hline 36 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{core/tx\textunderscore{}journal.go}} \\ \hline 37 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{core/tx\textunderscore{}pool.go}} \\ \hline 38 & \textcolor[rgb]{0.2,0.2,0.2}{core/types/block.go} \\ \hline 39 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{core/types/block\textunderscore{}test.go}} \\ \hline 40 & \textcolor[rgb]{0.2,0.2,0.2}{core/types/transaction.go} \\ \hline 41 & \textcolor[rgb]{0.2,0.2,0.2}{core/vm/evm.go} \\ \hline 42 & \textcolor[rgb]{0.2,0.2,0.2}{core/vm/instructions.go} \\ \hline 43 & \textcolor[rgb]{0.2,0.2,0.2}{core/vm/interface.go} \\ \hline 44 & \textcolor[rgb]{0.2,0.2,0.2}{core/vm/runtime/runtime.go} \\ \hline 45 & \textcolor[rgb]{0.2,0.2,0.2}{crypto/crypto.go} \\ \hline 46 & \textcolor[rgb]{0.2,0.2,0.2}{crypto/x/x.go} \\ \hline 47 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{crypto/x/x\textunderscore{}test.go}} \\ \hline 48 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{eth/api\textunderscore{}backend.go}} \\ \hline 49 & \textcolor[rgb]{0.2,0.2,0.2}{eth/backend.go} \\ \hline 50 & \textcolor[rgb]{0.2,0.2,0.2}{eth/config.go} \\ \hline 51 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{eth/db\textunderscore{}upgrade.go}} \\ \hline 52 & \textcolor[rgb]{0.2,0.2,0.2}{eth/downloader/downloader.go} \\ \hline 53 & \textcolor[rgb]{0.2,0.2,0.2}{eth/handler.go} \\ \hline 54 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{eth/handler\textunderscore{}test.go}} \\ \hline 55 & \textcolor[rgb]{0.2,0.2,0.2}{eth/sync.go} \\ \hline 56 & \textcolor[rgb]{0.2,0.2,0.2}{ethdb/database.go} \\ \hline 57 & \textcolor[rgb]{0.2,0.2,0.2}{internal/webext/webext.go} \\ \hline 58 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{les/api\textunderscore{}backend.go}} \\ \hline 59 & \textcolor[rgb]{0.2,0.2,0.2}{les/backend.go} \\ \hline 60 & \textcolor[rgb]{0.2,0.2,0.2}{les/fetcher.go} \\ \hline 61 & \textcolor[rgb]{0.2,0.2,0.2}{les/handler.go} \\ \hline 62 & \textcolor[rgb]{0.2,0.2,0.2}{light/lightchain.go} \\ \hline 63 & \textcolor[rgb]{0.2,0.2,0.2}{miner/agent.go} \\ \hline 64 & \textcolor[rgb]{0.2,0.2,0.2}{miner/miner.go} \\ \hline 65 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{miner/remote\textunderscore{}agent.go}} \\ \hline 66 & \textcolor[rgb]{0.2,0.2,0.2}{miner/unconfirmed.go} \\ \hline 67 & \textcolor[rgb]{0.2,0.2,0.2}{miner/worker.go} \\ \hline 68 & \textcolor[rgb]{0.2,0.2,0.2}{node/config.go} \\ \hline 69 & \textcolor[rgb]{0.2,0.2,0.2}{node/node.go} \\ \hline 70 & \textcolor[rgb]{0.2,0.2,0.2}{pp/discover/node.go} \\ \hline 71 & \textcolor[rgb]{0.2,0.2,0.2}{pp/discover/udp.go} \\ \hline 72 & \textcolor[rgb]{0.2,0.2,0.2}{pp/server.go} \\ \hline 73 & \textcolor[rgb]{0.2,0.2,0.2}{params/bootnodes.go} \\ \hline 74 & \textcolor[rgb]{0.2,0.2,0.2}{params/config.go} \\ \hline 75 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{params/protocol\textunderscore{}params.go}} \\ \hline 76 & \textcolor[rgb]{0.2,0.2,0.2}{params/version.go} \\ \hline 77 & \textcolor[rgb]{0.2,0.2,0.2}{rpc/server.go} \\ \hline 78 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{tests/block\textunderscore{}test\textunderscore{}util.go}} \\ \hline 79 & \textcolor[rgb]{0.2,0.2,0.2}{tests/init.go} \\ \hline 80 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{tests/state\textunderscore{}test\textunderscore{}util.go}} \\ \hline 81 & \textcolor[rgb]{0.2,0.2,0.2}{\texttt{tests/vm\textunderscore{}test\textunderscore{}util.go}} \\ \hline \end{longtable} \end{small} Source Of Truth --- CertiK used the following source of truth to enhance the understanding of WaltonChain's systems: 1. WaltonChain Whitepaper\footnote{ Whitepaper: \url{https://www.waltonchain.org/Uploads/2019-04-25/5cc1721e671c9.pdf}} 2. WaltonChain Website\footnote{ Website: \url{https://www.waltonchain.org/}} 3. WaltonChain Github\footnote{ Github: \url{https://github.com/WaltonChain/WaltonChain_Gwtc_Src}} 4. WaltonChain Business Process\footnote{ Business Process: \url{https://www.waltonchain.org/en/sys/cate/48.html}} All listed sources act as specification. For any inconsistency discovered between the actual code behavior and the specification, CertiK would consult with the WaltonChain team for further discussion and confirmation. ## Blockchain audit checklist The blockchain source code audit will conduct and focus on answering the following listed areas, checkpoints and harm level. ($\checkmark$ indicates satisfaction; $\times$ indicates unsatisfaction; $-$ indicates partial satisfaction) ### **Key Management** *Ensuring the all key generation meet following checkpoints:* - [$\checkmark$] Private Key & Mnemonic Generation Correctness - [$\checkmark$] Private Key & Mnemonic Storage Safety Management - [$\checkmark$] Private Key & Mnemonic Confidentiality ### **Cryptography** *Ensuring the correctness and fairness of the hash functions, signatures, and keys generation* - [$\checkmark$] Random hash algorithm correctness in terms of hash function, and signature - [$\checkmark$] Hash generation within normal distribution probability ### **Consensus** *Ensuring the consensus model design, and model correctness as described in the Source of trusts list* - [$-$] Consensus model design rationality - [$-$] Consensus implementation correctness as described design ### **RPC** *Ensuring the RPC interface, port, and exposure with restricted access for preventing vulnerability signature attack, and sensitive information leaking* - [$\checkmark$] Sensitive information & interface accessibility ### **Chain Processing** *Ensuring the chain is processing and synchronizing with longest selection and switching algorithm appropriately.* - [$\checkmark$] Longest Chain selection and switching algorithm - [$\checkmark$] Chain synchronization logic/workflow correctness ### **Block Processing** *Ensuring the block hash function, signature, and mertkle tree structure correctness for malicious activities caused by faking block data* - [$-$] Block processing resource limitation as for orphan block pool, verification calculation etc - [$\checkmark$] Block signatures and merkle tree root construction correctness ### **Transaction Processing** *Ensuring the correctness of transaction process for minimizing the chances of lost transaction, low cost DoS attack, double spending attack, and replay attack* - [$\checkmark$] Transaction fee model design - [$\checkmark$] Transaction processing logic & workflow - [$\checkmark$] Hash collision ### **Misc** - [$\times$] Compatibility with latest go-version. This is always a good practice for keeping programming language to the latest version, it benefits with toolchain, compiler, packages and standard library new features and fixes. - [$\times$] Update the vendor folders for obtaining the latest version of the third party libraries with its new features and bug fix updates. - [$\times$] High Test Coverage. Implementing the unit test as many as possible for ensuring function behavior is meeting its specification. A lot of time, unit-test can discover many unexpected vulnerabilities at the early stage of product release before the lost. - [$\times$] Provide System testing or End-to-End testing scripts. Strongly recommend to development some test workflows and scenarios covering the critical business process for capturing the errors at the beginning. This also would benefit for software update and release process later. - [$-$] Project github documentation with latest update. Strongly recommend update the project Readme file for better guideline to other audiences. - [$-$] Whitepaper and technical design documentation with latest update. Strongly recommend update the whitepaper and technical design documents with the latest changes make due to the business model and requirement changes. Golang Finding --- ### Go Version Incompatible * $\boxed{\text{MINOR}}$ According to the project `Readme.md`, WaltonChain support Go1.7+, however the current project won't be able to compile with Go1.10+. - $\boxed{\text{WALTONCHAIN}}$: For now, the project supports w/ Go1.7 && Go1.8. Other go version supports will be corporate later. ``` Go version 1.10+ compatible issue: While installing the WaltonChain with go version 1.10+, below error return: # github.com/wtc/go-wtc/vendor/github.com/rjeczalik/notify vendor/github.com/rjeczalik/notify/watcher_fsevents_cgo.go:51:52: cannot use nil as type _Ctype_CFAllocatorRef in assignment vendor/github.com/rjeczalik/notify/watcher_fsevents_cgo.go:162:40: cannot use nil as type _Ctype_CFAllocatorRef in argument to _Cfunc_CFStringCreateWithCStringNoCopy vendor/github.com/rjeczalik/notify/watcher_fsevents_cgo.go:162:55: cannot use nil as type _Ctype_CFAllocatorRef in argument to _Cfunc_CFStringCreateWithCStringNoCopy vendor/github.com/rjeczalik/notify/watcher_fsevents_cgo.go:163:40: cannot use nil as type _Ctype_CFAllocatorRef in assignment ``` The issue can be address by updating the vendor folder with following changes: ``` line51: var source = C.CFRunLoopSourceCreate(C.kCFAllocatorDefault, 0, &C.CFRunLoopSourceContext{ perform: (C.CFRunLoopPerformCallBack)(C.gosource), }) line:162: p := C.CFStringCreateWithCStringNoCopy(C.kCFAllocatorDefault, C.CString(s.path), C.kCFStringEncodingUTF8, C.kCFAllocatorDefault) line:163: path := C.CFArrayCreate(C.kCFAllocatorDefault, (*unsafe.Pointer)(unsafe.Pointer(&p)), 1, nil) ``` * $\boxed{\text{MINOR}}$ `build/ci.go` Consider using [library](https://github.com/hashicorp/go-version) for go-version checking. * $\boxed{\text{MINOR}}$ `accounts/keystore/key.go` Missing the sanity check for `keyAddr == nil` and `keyAddrr[:8]` possible lead to index out of range Update the comment for `keyFileName`, the behaviour is not same as described. ### Consensus * $\boxed{\text{DISCUSSION}}$ WaltonChain is implementing a customize X11 algorithm, how is the algorithm works & its purpose? * $\boxed{\text{WaltonChain}}$: The main difference between `myX11` and other X11 algorithms that on the market is the algorithm replacement policy. The intention of this design is to keep the hash-rate at a mineable rate for attracting more miners to work on the at the beginning. Due to the Waltonchain business intention, it requires with a block rate of 30s. The replacement policy work as following: - The 11 algorithms are classifying into two subsets: `metaDiscard` & `metaReplace`. - If any of the selected hash function is contained within the `metaDiscard`, then it will replace by hash function from `metaReplace`. * $\boxed{\text{INFO}}$ The x11 hash functions relative time can be projected as following: - A=blake - B=bmw - C=groestl - D=jh - E=keccak - F=skein - G=luffa - H=cubehash - I=shavite - J=simd - K=echo \begin{figure} \centering \includegraphics[width=0.6\textwidth]{/Users/connielam/Workspace/certik-contracts/contracts/waltonchain/figures/relativetime.png} \caption{Relative Time per Hash Algorithm} \label{fig:RelativeTimePerHashAlgorithm} \end{figure} - Individual Hash function time consumption (ordering from low to high): - skein(F), bmw(B), blake(A), keccak(E), shavite(I), cubehash(H), jh(D), luffa(G), echo(K), simd(J), groestl (C) - `metaDiscard`: **CFIJK**, - `metaReplace`: **AHDGEB** Over the 7 days, the [`miningpoolstats`](https://miningpoolstats.stream/waltonchain) statistic indicated that the hashrate is in a average range of 235.33 GH/s * $\boxed{\text{DISCUSSION}}$ Why use header number hash X11 order? - $\boxed{\text{WaltonChain}}$ The reason of using the header number hash is because of someone could change other fields in the header (e.g. extra) to manipulate X11 order. keccak256 (change extra field) vs. X11 (increasing nonce). * $\boxed{\text{DISCUSSION}}$ How is the POS consensus protocol work in WaltonChain? - $\boxed{\text{WaltonChain}}$ Compare to traditional POS, WaltonChain's POS offers mining with staking economic model. For those who staking WTC token miners, they can obtain more rewards & 75% discount regarding the difficulty while mining, also no slashing, penalty More more detail of the **Waltonchain Progressive Mining Reward Program** can be found [here](https://www.waltonchain.org/doc/Waltonchain_Progressive_Mining_Reward_Program.pdf). * $\boxed{\text{DISCUSSION}}$ Why the coinage is removed from the codebase? * $\boxed{\text{WaltonChain}}$ The concept of coinage is deprecated after the malicious attack. The attackers manipulate the coinage for receiving more mining rewards. The issue is addressed, and the patch has been released. For more, the mainnet upgrade press released by June 29, 2019 can be found [here](https://medium.com/@Waltonchain_EN/announcement-on-waltonchain-mainnet-upgrade-at-block-175-366-9534c8ac0a3b). * $\boxed{\text{INFO}}$ The [change commit](https://github.com/WaltonChain/WaltonChain_Gwtc_Src/commit/98a829e52a4c7ba3d8d5938e8025362132280717#diff-e226000964244889df127e5b44dbf192R507) between [v1.1.2](https://github.com/WaltonChain/WaltonChain_Gwtc_Src/releases/tag/v1.1.2) to [v1.1.3](https://github.com/WaltonChain/WaltonChain_Gwtc_Src/releases/tag/v1.1.2): 1. Fixed the usage of sha256 on header.Number in `consensus/ethash/consensus.go` and `consensus/ethash/sealer.go`. (edited) 2. Divided the calculation of difficulty into two stages in `consensus/ethash/consensus.go`. (edited) 3. Reduced the minGasLimit to be compared with in `params/protocol_params.go` and updated the comparison in `consensus/ethash/consensus.go` and `core/block_validator.go`. * $\boxed{\text{DISCUSSION}}$: The codebase unused the uncle block, however in ethereum `uncle block` means more than just a reward block, it also benefit for minimizing the centralize problem that POW will facing in the long run. - [Reference] https://blog.ethereum.org/2014/07/11/toward-a-12-second-block-time/ - **`consensus/ethash/algorithm.go`** * $\boxed{\text{MAJOR}}$`Sqrt()`, is not behavoiur as the function named, please renamed the function to nth-Root(), Please review this [reference](https://steemit.com/tutorial/@gopher23/power-and-root-functions-using-big-float-in-golang) for calculating a precise n-th root function. - $\boxed{\text{WaltonChain}}$ the function is taking 6 times sqrt root of a given number. * $\boxed{\text{MAJOR}}$`Sqrt()`, using big.Int might caused issue in number precision, consider using the big.Float instead. * $\boxed{\text{MINOR}}$ `log2()` Recommend to removed if not used. The function behavior as returning log(n) + 1 ### Smart Contract The smart contracts are designed for transfering the facilitator of data transfers between the various sidechains to WaltonChain parent chain. Side chain collect transactions and bundle 50 txns and pushing to parent chain. The WTC tokens are required for the transactions to be complete. - `Ballot.sol`: \newline 737383ee58c5511b3188fcc8675f528b6b6cf482214a7fd41a72561ce8c2f5ca - `PermissionManageContract.sol`: \newline 7fd97c86123affb44eac2b2fb05dc436dc29617cc9a7783dce3d4dfb855dcd35 - `SubchainManageContract.sol`: \newline 339f50199d2964190be0d254097512ba956e0fd3575ab7cae969f3f0cc8056e9 - `TemplateManageContract.sol`: \newline c2cac36efd30ba049432fe2dcf302549be2988344913c5e1e91a58d22a605f7a **Ballot.sol** * $\boxed{\text{INFO}}$ Consider using `SafeMath` Library for all math operations to prevent overflow when working with `uint` - `vote()`: line 67, line 69 - `delegete()`: line 94, line 99, line 101 * $\boxed{\text{INFO}}$: Recommend to separate a private function for handling the conditions for `passCount` & `vetoCount`. In this case, the below function can be reused in both `vote()` & `delegate()` ``` function updateIsPass() private { if (passCount >= passThreshold) { isPass = true; } else if (vetoCount >= vetoThreshold) { isPass = false; } } ``` * $\boxed{\text{INFO}}$: `judgeVoterThreshold()` recommend to use safe math. * $\boxed{\text{INFO}}$: `CommonProposal()` Recommend to ensure sender is not a zero address, feature is a non-empty string. * $\boxed{\text{MINOR}}$: Consider declaring functions as `external`, when there is no need for an internal access from contract itself. It could benefit for gas consumption. `public` modifier consumed 496 gas vs `external` consumed 261 gas only This is due to the fact that Solidity copies arguments to memory on a public function, while external read from calldata which a cheaper than memory allocation. * Vote.vote * Vote.delegate * Proposal.voterVote * Proposal.voterDelegate * Proposal.judgmentVote * Proposal.judgmentDelegate * Ballot.addJMNProposal * Ballot.addSMNProposal * Ballot.systemProposalCount * Ballot.commonProposalCount * Ballot.startSystemCallback * Ballot.stopSystemCallback * Ballot.updateSystemCallback * Ballot.start * Ballot.stop * Ballot.update * $\boxed{\text{INFO}}$: Recommend implementing another private function for different conditions handling and code reusability in both `vote()` & `delegate()` ``` function updateIsPass() private { if (passCount >= passThreshold) { isPass = true; } else if (vetoCount >= vetoThreshold) { isPass = false; } } ``` * $\boxed{\text{MINOR}}$: Potential lead to Re-entrancy vulnerability of following functions and consider applying following changes: - `voterVote()` ``` function voterVote(bool _decision) public OnlyVote { updateStageVote(); vote.vote(msg.sender, _decision); } ``` - `voterDelegate()` ``` function voterDelegate(address _delegateAddress) public OnlyVote { updateStageVote(); vote.delegate(msg.sender, _delegateAddress); } ``` - `judgementVote()` ``` function judgmentVote(bool _decision) public OnlyJudgment { updateStageJudgment(); judgment.vote(msg.sender, _decision); } ``` - `judgementDelegate()` ``` function judgmentDelegate(address _delegateAddress) public OnlyJudgment { updateStageJudgment(); judgment.delegate(msg.sender, _delegateAddress); } ``` * $\boxed{\text{MINOR}}$: `addSystemProposal()`,`addJMNProposal()`, `addSMNProposal` be more effective swap the mapping and push operation. ``` systemProposalMapping[address(systemProposal)] = systemProposals.length; systemProposals.push(systemProposal); ``` **PermissionManageContract.sol** * $\boxed{\text{MINOR}}$: `constructor()` Consider adding zero address checking for `_owner`. * $\boxed{\text{INFO}}$: Consider declare below functions to external for gas optimization: - PermissionManager.addJMN - PermissionManager.addSMN - PermissionManager.jmnCount - PermissionManager.smnCount - PermissionManager.deleteJMN - PermissionManager.deleteSMN - PermissionManager.addWorker - PermissionManager.deleteWorker - PermissionManager.workerSize - PermissionManager.getSMNAddress **SubchainManageContract.sol** * $\boxed{\text{INFO}}$ `emptyEndorsement`: Consider to remove never used variable. * $\boxed{\text{MINOR}}$: `addEndorsement()`: Possible contract may occur reentrancy vulnerabilities, consider to use check-effect-interact pattern to minimize the impact: - Below state variables changed, after the external call(s) to \newline `ITemplateManager.getContractTemplate()` && \newline `ITemplateManager.deployContract()`. * $\boxed{\text{MINOR}}$ `endorsementContractAbi`: Is it possible return an empty data * $\boxed{\text{MINOR}}$ `endorsementContractAddress`: Is it possible return a zero address * $\boxed{\text{INFO}}$: Consider declare below functions to external for gas optimization: - SubchainManager.addSubchain - SubchainManager.addEndorsement - SubchainManager.getSubchain - SubchainManager.getEndorsement - SubchainManager.subchainSize - SubchainManager.endorsementSize - SubchainManager.addSMNAttention - SubchainManager.smnAttentionSize - SubchainManager.deleteSMNAttention **TemplateManageContract.sol** * $\boxed{\text{INFO}}$: `constructor()`: Consider checking the `_owner` is not a zero address. ``` constructor(address _owner) public { require(_owner != address(0), "_owner is a zero address"); owner = _owner; contractTemplates.push(emptyContractTemplate); } ``` * $\boxed{\text{MAJOR}}$: `deployContract()`: Consider adding code for ensuring the a new contract is created. ``` assembly { deployContractAddress := create(0, add(bytecodeWithAddress, 0x20), mload(bytecodeWithAddress)) let codeSize := extcodesize(deployContractAddress) if eq(codeSize, 0) { revert(0, 0) } } ``` * $\boxed{\text{MINOR}}$: `addEndorsement()`: Possible contract may occur reentrancy vulnerabilities, consider to use check-effect-interact pattern to minimize the impact: Below state variables changed, after the external call(s) to [`ITemplateManager.getContractTemplate()` && `ITemplateManager.deployContract()`]: endorsements subchainEndorsementsMapping - `endorsementContractAbi`: Is it possible return an empty data - `endorsementContractAddress`: Is it possible return a zero address * $\boxed{\text{INFO}}$: Consider declare below functions to external for gas optimization: - SubchainManager.addSubchain - SubchainManager.addEndorsement - SubchainManager.getSubchain - SubchainManager.getEndorsement - SubchainManager.subchainSize - SubchainManager.endorsementSize - SubchainManager.addSMNAttention - SubchainManager.smnAttentionSize - SubchainManager.deleteSMNAttention ## Best practice Smart contract development requires a particular engineering mindset. A failure in the initial construction can be catastrophic, and changing the project after the fact can be exceedingly difficult. To ensure success and to avoid the challenges above smart contracts should here to best practices at their conception. Below, we summarized a checklist of key points that help to indicate a high overall quality of the current WaltonChain cross-chain project. ($\checkmark$ indicates satisfaction; $\times$ indicates unsatisfaction; $-$ indicates inapplicablility) ### **General** - [$\checkmark$] Corrent environment settings, e.g. compiler version, test framework - [$\checkmark$] No compiler warnings - [$\checkmark$] Provide error message along with `assert` - [$\times$] Use events to monitor contract activities - [$\checkmark$] Correct time dependency ### **Solidity Specific** - [$-$] Safe external call - [$-$] No reentrancy pattern - [$\checkmark$] Correct handling of integer overflow and underflow - [$\checkmark$] Correct visibility for state variables ### **Privilege Control** - [$\checkmark$] Provide pause functionality for control and emergency handling - [$\checkmark$] Provide time buffer between certain operations - [$\checkmark$] Provide proper access control for functions - [$\checkmark$] Establish rate limit for certain operations - [$\checkmark$] Restrict access to sensitive functions - [$\checkmark$] Restrict permission to contract destruction ### **Documentation** - [$\checkmark$] Provide project README and execution guidance - [$\checkmark$] Provide inline comment for function intention - [$\checkmark$] Provide instruction to initialize and execute the test files ### **Testing** - [$\times$] Provide migration scripts - [$\times$] Provide test scripts and coverage for potential scenarios Overall we found the smart contracts to follow good practices. With the final update of source code and delivery of the audit report, we conclude that the contract is structurally sound and not vulnerable to any classically known anti-patterns or security issues. The audit report itself is not necessarily a guarantee of correctness or trustworthiness, and we always recommend to seek multiple opinions, keep improving the codebase, and more test coverage and sandbox deployments before the mainnet release.
{"metaMigratedAt":"2023-06-14T23:04:49.850Z","metaMigratedFrom":"Content","title":"Waltonchain","breaks":true,"contributors":"[{\"id\":\"b10a465c-6ca4-44e7-a5f0-0764712f898c\",\"add\":84404,\"del\":54192},{\"id\":\"5df1c34f-a306-487e-9854-1f45e00dae19\",\"add\":296,\"del\":95}]"}
Expand menu