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}]"}