[renBTC] ERC20 Token Smart Contract Technical Assessment

[hexonaut]

[renBTC] ERC20 Token Smart Contract Technical Assessment

General Information

• Symbol : renBTC
• Address(es) :
  • RenBTC (Token Proxy): 0xEB4C2781e4ebA804CE9a9803C67d0893436bB27D
  • RenERC20LogicV1 (Implementation): 0xe2d6cCAC3EE3A21AbF7BeDBE2E107FfC0C037e80
• Deployment Date :
  • RenBTC: Mar-24-2020 11:02:24 PM +UTC
  • RenERC20LogicV1: Mar-25-2020 03:45:59 AM +UTC
• Total supply : 1678237219367 WAD (16,782 units)
• Developers allotment : None
• Collateral Onboarding Application: [renBTC] MIP6 Collateral Application
• Project website : https://renproject.io/
• Github repository :
  • https://github.com/renproject
• Can use existing MCD collateral type adapter?
  Yes, the GemJoin5 adapter.

Risk Summary

• Does the contract implement the ERC20 token standards? Yes.
• Risk analysis : MEDIUM.

Technical Information

• Compiler version : v0.5.16+commit.9c3226ce
• Decimals : 8
• Overflow checks : Yes, the contract uses the SafeMath library for uint operations.
• Mitigation against allowance race-condition : Yes, the contract implements increaseAllowance and decreaseAllowance to get around this issue.
• Upgradeable contract patterns : Yes.
• Access control or restriction lists : No.
• Non-standard features or behaviors : Yes.
  • Two EOAs Mint Authority and Owner can mint renBTC.

Formal Verification Considerations:

• Does transfer have simple semantics? Yes.
• Does transferFrom have simple semantics? Yes.
• Can balances be arbitrarily modified by some actor? No.
• Are there any external calls? No.

Testnet Information

• renBTC is deployed on Kovan. List of relevant addresses here.

Contract Logic Summary

Administrative Addresses

• ProxyAdmin (Admin of Token and BTCGateway contracts): 0xDf1D8eD27C54bBE5833320cf5a19fd9E73530145 [Contract]
• Timelock (ProxyAdmin.owner and BTCGateway.owner): 0xfE43c3a01102EDA295333F5Ad6CC747f03314fdb [Contract]
• Gnosis Multisig (Timelock.admin): 0xc49d7d07f1b21168a89d686F3E65BC07142250ef [Contract]
• BTCGateway (Token Owner): 0xe4b679400F0f267212D5D812B95f58C83243EE71 [Contract]
• BTCGateway Implementation: 0x402ec534BaF9e8Dd2968c57fDea368f3856460d6 [Contract]
• BTCGateway.mintAuthority: 0x7f64e4E4b2D7589Eb0ac8439C0e639856aeCEEe7 [EOA]
• BTCGateway.owner: 0xfE43c3a01102EDA295333F5Ad6CC747f03314fdb [Contract]

The minting is controlled by an EOA - the Mint Authority.

The proxy implementation can be changed arbitrarily by the timelocked multsig after a 7 day delay.

Inheritance Structure

renBTC uses a bunch of standard OpenZeppelin contracts for the Proxy and ERC20 implementation.

Contract Risk Summary

This is a MEDIUM risk contract. The ERC20 functions are implemented to industry standard and there are checks to prevent over/underflows. The risk comes from minting being controlled by a single EOA, and the upgradable contract pattern. However, implementation changes are behind a 7 day timelock controlled by a multisig, so there is time to react via governance executive.

Supporting Materials

Architecture Diagram

Inheritance Diagram

Sūrya’s Description Report

Files Description Table

File Name	SHA-1 Hash
renBTC.sol	fc10e6ea816a5b894602dddea5f6c886f199b8f4

Contracts Description Table

Contract	Type	Bases
└	Function Name	Visibility	Mutability	Modifiers
Initializable	Implementation
└	isConstructor	Private
Context	Implementation	Initializable
└		Internal
└	_msgSender	Internal
└	_msgData	Internal
Ownable	Implementation	Initializable, Context
└	initialize	Public		initializer
└	owner	Public		NO❗️
└	isOwner	Public		NO❗️
└	renounceOwnership	Public		onlyOwner
└	transferOwnership	Public		onlyOwner
└	_transferOwnership	Internal
Proxy	Implementation
└		External		NO❗️
└	_implementation	Internal
└	_delegate	Internal
└	_willFallback	Internal
└	_fallback	Internal
OpenZeppelinUpgradesAddress	Library
└	isContract	Internal
BaseUpgradeabilityProxy	Implementation	Proxy
└	_implementation	Internal
└	_upgradeTo	Internal
└	_setImplementation	Internal
UpgradeabilityProxy	Implementation	BaseUpgradeabilityProxy
└		Public		NO❗️
BaseAdminUpgradeabilityProxy	Implementation	BaseUpgradeabilityProxy
└	admin	External		ifAdmin
└	implementation	External		ifAdmin
└	changeAdmin	External		ifAdmin
└	upgradeTo	External		ifAdmin
└	upgradeToAndCall	External		ifAdmin
└	_admin	Internal
└	_setAdmin	Internal
└	_willFallback	Internal
InitializableUpgradeabilityProxy	Implementation	BaseUpgradeabilityProxy
└	initialize	Public		NO❗️
InitializableAdminUpgradeabilityProxy	Implementation	BaseAdminUpgradeabilityProxy, InitializableUpgradeabilityProxy
└	initialize	Public		NO❗️
IERC20	Interface
└	totalSupply	External		NO❗️
└	balanceOf	External		NO❗️
└	transfer	External		NO❗️
└	allowance	External		NO❗️
└	approve	External		NO❗️
└	transferFrom	External		NO❗️
SafeMath	Library
└	add	Internal
└	sub	Internal
└	sub	Internal
└	mul	Internal
└	div	Internal
└	div	Internal
└	mod	Internal
└	mod	Internal
ERC20	Implementation	Initializable, Context, IERC20
└	totalSupply	Public		NO❗️
└	balanceOf	Public		NO❗️
└	transfer	Public		NO❗️
└	allowance	Public		NO❗️
└	approve	Public		NO❗️
└	transferFrom	Public		NO❗️
└	increaseAllowance	Public		NO❗️
└	decreaseAllowance	Public		NO❗️
└	_transfer	Internal
└	_mint	Internal
└	_burn	Internal
└	_approve	Internal
└	_burnFrom	Internal
ERC20Detailed	Implementation	Initializable, IERC20
└	initialize	Public		initializer
└	name	Public		NO❗️
└	symbol	Public		NO❗️
└	decimals	Public		NO❗️
Claimable	Implementation	Initializable, Ownable
└	initialize	Public		initializer
└	transferOwnership	Public		onlyOwner
└	claimOwnership	Public		onlyPendingOwner
Address	Library
└	isContract	Internal
└	toPayable	Internal
└	sendValue	Internal
SafeERC20	Library
└	safeTransfer	Internal
└	safeTransferFrom	Internal
└	safeApprove	Internal
└	safeIncreaseAllowance	Internal
└	safeDecreaseAllowance	Internal
└	callOptionalReturn	Private
CanReclaimTokens	Implementation	Claimable
└	initialize	Public		initializer
└	blacklistRecoverableToken	Public		onlyOwner
└	recoverTokens	External		onlyOwner
ERC20WithRate	Implementation	Initializable, Ownable, ERC20
└	initialize	Public		initializer
└	setExchangeRate	Public		onlyOwner
└	exchangeRateCurrent	Public		NO❗️
└	_setRate	Internal
└	balanceOfUnderlying	Public		NO❗️
└	toUnderlying	Public		NO❗️
└	fromUnderlying	Public		NO❗️
ERC20WithPermit	Implementation	Initializable, ERC20, ERC20Detailed
└	initialize	Public		initializer
└	permit	External		NO❗️
RenERC20LogicV1	Implementation	Initializable, ERC20, ERC20Detailed, ERC20WithRate, ERC20WithPermit, Claimable, CanReclaimTokens
└	initialize	Public		initializer
└	mint	Public		onlyOwner
└	burn	Public		onlyOwner
└	transfer	Public		NO❗️
└	transferFrom	Public		NO❗️
RenBTC	Implementation	InitializableAdminUpgradeabilityProxy
RenZEC	Implementation	InitializableAdminUpgradeabilityProxy
RenBCH	Implementation	InitializableAdminUpgradeabilityProxy

Legend

Symbol	Meaning
	Function can modify state
	Function is payable

[BrianMcMakerDAO]

@hexonaut

Due to the lack of time lock and the control being an EOA instead of a multi-sig, I am giving this token a HIGH risk.

Thanks for bringing this up, Sam. I’ll add that after looking at the Token Proxy, the _implementation() call has been made internal on the contract, so there’s no way to mitigate this risk with an implementation check in the adapter, either.

[MaxRoszko]

We will in the next 24h start the process of moving over the control to a timelock contract. We’ll be using this audited timelock by Compound, and have the minimum delay set at 7 days.

The reason for not implementing a timelock before is out of an abundance of caution, in case we needed to quickly implement a fix or a change. On the other hand our confidence in the security of the system is growing every day and we think that should be reflected in the token contracts as well, so we have no blocker to changing control over to a timelock.

[hexonaut]

Appreciate the timely response @MaxRoszko. A 7 day timelock is definitely a lot better than what exists now.

What are your thoughts on switching to a multisig to control the implementation as well? It’s a lot of power for a single EOA, and we cannot automatically deactivate on implementation change due to the transparent proxy.

[MaxRoszko]

Yes we’ll control the timelock through a standard Gnosis Multisig for now, with plans of decentralizing the multisig setup next year (perhaps through a different system), as we hand more governance over to the REN holders.

[hexonaut]

Great, thanks for addressing these concerns. I’ll update the assessment after these changes are made.

[MaxRoszko]

I’ll keep you guys posted in this thread!

[MaxRoszko]

This has been resolved now, all Ren gateways (so not just the renBTC contract but renZEC, renFIL etc.) are now owned by a timelock contract, and the timelock contract is owned by a Gnosis Multisig!

[hexonaut]

Just checked this again, and it looks like the EOA 0x33024cfb7af11a7cb12ab0dedefc5dd5f430381f is still in control of the ProxyAdmin.

The ADMIN_SLOT position on RenBTC points to RenProxyAdmin which has its owner() set to 0x33024cfb7af11a7cb12ab0dedefc5dd5f430381f. Same with BTCGateway.

[MaxRoszko]

Sorry about this, there was a failed transaction last time that was unnoticed, here is the new successful one: https://etherscan.io/tx/0x8f2b1be5705ea00841947971ea77d9d1e985d88eaac0bc687ce95e5d9b7dea86

[hexonaut]

Appreciate you making those changes @MaxRoszko. I’ve downgraded the risk to MEDIUM and updated the post to match the new setup.