Jelly-market
Published in

Jelly-market

Jelly Contracts Audit

The Jelly team asked ABDK consulting to review and audit its protocol. They looked at the code for two weeks and published the results.

The audited commit is 8708e0b224ef0058f560d4d746de9d88188c0b86 the Solidity contract can be found here.

Before moving to the list of issues found in the project, let’s make some introduction about it.

The concept

  • Jelly is a DeFi product built to achieve interoperability across chains by enabling value transfer between the different networks.
  • Jelly enables cross-chain atomic swaps between coins from heterogenous blockchains.
  • Jelly is a decentralized, non-custodial and permissionless app powered by Hash Time Locked Contracts.

Jelly allows cross-chain atomic swaps between the Bitcoin, Ethereum and Aeternity blockchains. More networks such as TRON, EOS, NEO and Bitcoin Cash will be added in future. Additionally, there are separate trading pairs between coins (like BTC) and tokens (ERC-20 like DAI) which we believe will be crucial for the Decentralized Finance (DAI-BTC, DAI-AE, DAI-TRX, etc.). It will help DeFi to go beyond Ethereum and spread across other blockchains and thus expand its market.

Apart from swapping coins, Jelly also offers the opportunity to provide liquidity and earn profit. Market Makers can gain profit from price spreads with no restrictions and the ability to get out of the protocol at any time. This is possible due to the instant OTC (Over-The-Counter) market making software developed by our team and called Buttler.
More info about Butler will be available soon.

Audit Results

Jelly team applied several fixes based on the audit recommendations. A number of issues were identified, and all the important ones have been fixed in the new version. Those remaining do not affect the security of the contract.

Issues found in the Hash Time Locked Contracts

1.Critical

Line 86: if the function newContract is called twice with the same parameters and msg.value, then the same ID will be generated. The information about the previous contract with such ID will be lost. The ether associated with the previous contract will become inaccessible.

Answer: This cannot happen while using the application. One must manually and intentionally send the same parameters. However, this issue was fixed.

2. Suboptimal code

Line 2: there should be ^0.5.0 instead of >=0.5.0. It would look more common.

Answer: This issue was fixed.

Line 17–18: the storing the full strings could be gas consuming. Consider storing only the hashes of these strings.

Answer: This is left as the strings are expected to be short (e.g.. ETH, BTC, AE)

Line 21, 29: Withdraw and Refund events are contain redundant data that can be retrieved from NewContract event by using the id.

Answer: This is left as applications already use this interface.

Line 24, 41: it might be better to index hashLock not sender.

Answer: This is left as applications already use this interface.

Line 30: probably, hashLock can be used for the id as it should never occur twice.

Answer: This is left as non-important.

Line 49, 60, 155: the memory modifier may cause the whole structure will be read into the memory, including the fields that are not needed for this modifier. Consider using storage modifier instead.

Answer: This issue was fixed.

Line 116, 124: the contracts[id] already calculated inside withdrawable and refundable modifiers. Consider refactoring.

Answer: This issue was fixed.

Line 126, 127: the c.sender is the same as the msg.sender, but the msg.sender is cheaper to access.

Answer: Left as non-important.

Line 119, 127: the c.hashLock was already read from the storage inside withdrawable modifier. Consider refactoring.

Answer: This issue was fixed.

Line 131: the function getContract seems redundant as the function contracts is already public.

Answer: This issue was fixed.

Line 137: the contracts[id].status != SwapStatus.INVALID relays on non obvious fact that INVALID is the default value for SwapStatus enum.

Answer: This issue was fixed.

Line 154: the code could be made more readable and less error prone by replacing SwapStatus enum with a collection of separate constants.

Answer: This issue was fixed.

3. Unclear Behaviour

Line 68: the variable outputAmount is not used and is only logged.

Answer: This was found to be on purpose.

4. Documentation and other issues

Line 7: the EXPIRED is the status between the ACTIVE and the REFUNDED, which is not obvious. Some comment is needed.

Answer: This issue was fixed.

Line 120, 128: functions withdraw and refund always return true.

Answer: This issue was fixed.

Line 159: the condition tempContract.expiration < block.timestamp can be inaccurate when queried out of execution.

Answer: This is left out as OK, but an application should take that into account when querying the contract.

Final Results

No critical or serious issues were found in the new version of the code where the comments of the auditing consultants were taken into account.

Link to the audit: https://github.com/jelly-swap/jelly-eth-htlc/blob/master/HashTimeLock.%20Audit.pdf

We summarized all the issues in the following table:

For more information about Jelly:

Landing page: https://jelly.market/
App: https://app.jelly.market/swap
Whitepaper: https://jelly.market/documents/Jelly-Swap-Ecosystem.pdf
Github: https://github.com/jelly-swap
Twitter: https://twitter.com/jelly_swap
Telegram: https://t.me/jelly_swap

Jelly Logo

--

--

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store
Jelly

Jelly

Peer-to-Peer trades across different blockchains