-
Notifications
You must be signed in to change notification settings - Fork 12.4k
Azavalla feature/inheritable contract #680
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
eternauta1337
merged 25 commits into
OpenZeppelin:master
from
eternauta1337:azavalla-feature/inheritable-contract
Jan 18, 2018
Merged
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
4fe2157
Inheritable contract
azavalla 2a560ad
name fix
azavalla b709206
fixes
azavalla a613cd0
added example use-case for Inheritable.sol
azavalla a8afb20
Use Inherited event instead of OwnershipTransfered
azavalla d808e49
changed 'pronounce' for 'proclaim'
azavalla 80ae074
marked ownerLives as view
azavalla 560a855
further example documentation
azavalla 4332135
changed events names on inheritable test
azavalla c7807c5
changed view for constant
azavalla 5716492
setHeartbeatTimeout: public --> internal
azavalla e596046
owner can't be set as heir
azavalla 82c8512
changed Inherited event for OwnershipTransfered
azavalla 46736da
added test for SimpleSavingsWallet
azavalla c70ee93
[Inheritable.js] replace assertJump for expectThrow
azavalla fe712c6
[SimpleSavingsWallet.js] replace assertJump for expectThrow
azavalla 52b6181
renamed Inheritable --> Heritable
azavalla 51c2c50
[Heritable] ownerLives(): constant --> view
azavalla 5ea9bd4
[Heritable] added HeirOwnershipClaimed event
azavalla 3009553
Update test names and js style
eternauta1337 22c1403
Fix solidity style issue
eternauta1337 9d005b4
Explicit uint size
eternauta1337 c01203b
Index addreses in events and explicit uint size
eternauta1337 a5ea0af
Merge branch 'master' into azavalla-feature/inheritable-contract
eternauta1337 c8c0f21
Merge branch 'master' into azavalla-feature/inheritable-contract
eternauta1337 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| pragma solidity ^0.4.11; | ||
|
|
||
| import "../ownership/Heritable.sol"; | ||
|
|
||
|
|
||
| /** | ||
| * @title SimpleSavingsWallet | ||
| * @dev Simplest form of savings wallet whose ownership can be claimed by a heir | ||
| * if owner dies. | ||
| * In this example, we take a very simple savings wallet providing two operations | ||
| * (to send and receive funds) and extend its capabilities by making it Heritable. | ||
| * The account that creates the contract is set as owner, who has the authority to | ||
| * choose an heir account. Heir account can reclaim the contract ownership in the | ||
| * case that the owner dies. | ||
| */ | ||
| contract SimpleSavingsWallet is Heritable { | ||
|
|
||
| event Sent(address indexed payee, uint256 amount, uint256 balance); | ||
| event Received(address indexed payer, uint256 amount, uint256 balance); | ||
|
|
||
|
|
||
| function SimpleSavingsWallet(uint256 _heartbeatTimeout) Heritable(_heartbeatTimeout) public {} | ||
|
|
||
| /** | ||
| * @dev wallet can receive funds. | ||
| */ | ||
| function () public payable { | ||
| Received(msg.sender, msg.value, this.balance); | ||
| } | ||
|
|
||
| /** | ||
| * @dev wallet can send funds | ||
| */ | ||
| function sendTo(address payee, uint256 amount) public onlyOwner { | ||
| require(payee != 0 && payee != address(this)); | ||
| require(amount > 0); | ||
| payee.transfer(amount); | ||
| Sent(payee, amount, this.balance); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| pragma solidity ^0.4.11; | ||
|
|
||
|
|
||
| import "./Ownable.sol"; | ||
|
|
||
|
|
||
| /** | ||
| * @title Heritable | ||
| * @dev The Heritable contract provides ownership transfer capabilities, in the | ||
| * case that the current owner stops "heartbeating". Only the heir can pronounce the | ||
| * owner's death. | ||
| */ | ||
| contract Heritable is Ownable { | ||
| address public heir; | ||
|
|
||
| // Time window the owner has to notify they are alive. | ||
| uint256 public heartbeatTimeout; | ||
|
|
||
| // Timestamp of the owner's death, as pronounced by the heir. | ||
| uint256 public timeOfDeath; | ||
|
|
||
| event HeirChanged(address indexed owner, address indexed newHeir); | ||
| event OwnerHeartbeated(address indexed owner); | ||
| event OwnerProclaimedDead(address indexed owner, address indexed heir, uint256 timeOfDeath); | ||
| event HeirOwnershipClaimed(address indexed previousOwner, address indexed newOwner); | ||
|
|
||
|
|
||
| /** | ||
| * @dev Throw an exception if called by any account other than the heir's. | ||
| */ | ||
| modifier onlyHeir() { | ||
| require(msg.sender == heir); | ||
| _; | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * @notice Create a new Heritable Contract with heir address 0x0. | ||
| * @param _heartbeatTimeout time available for the owner to notify they are alive, | ||
| * before the heir can take ownership. | ||
| */ | ||
| function Heritable(uint256 _heartbeatTimeout) public { | ||
| setHeartbeatTimeout(_heartbeatTimeout); | ||
| } | ||
|
|
||
| function setHeir(address newHeir) public onlyOwner { | ||
| require(newHeir != owner); | ||
| heartbeat(); | ||
| HeirChanged(owner, newHeir); | ||
| heir = newHeir; | ||
| } | ||
|
|
||
| /** | ||
| * @dev set heir = 0x0 | ||
| */ | ||
| function removeHeir() public onlyOwner { | ||
| heartbeat(); | ||
| heir = 0; | ||
| } | ||
|
|
||
| /** | ||
| * @dev Heir can pronounce the owners death. To claim the ownership, they will | ||
| * have to wait for `heartbeatTimeout` seconds. | ||
| */ | ||
| function proclaimDeath() public onlyHeir { | ||
| require(ownerLives()); | ||
| OwnerProclaimedDead(owner, heir, timeOfDeath); | ||
| timeOfDeath = now; | ||
| } | ||
|
|
||
| /** | ||
| * @dev Owner can send a heartbeat if they were mistakenly pronounced dead. | ||
| */ | ||
| function heartbeat() public onlyOwner { | ||
| OwnerHeartbeated(owner); | ||
| timeOfDeath = 0; | ||
| } | ||
|
|
||
| /** | ||
| * @dev Allows heir to transfer ownership only if heartbeat has timed out. | ||
| */ | ||
| function claimHeirOwnership() public onlyHeir { | ||
| require(!ownerLives()); | ||
| require(now >= timeOfDeath + heartbeatTimeout); | ||
| OwnershipTransferred(owner, heir); | ||
| HeirOwnershipClaimed(owner, heir); | ||
| owner = heir; | ||
| timeOfDeath = 0; | ||
| } | ||
|
|
||
| function setHeartbeatTimeout(uint256 newHeartbeatTimeout) internal onlyOwner { | ||
| require(ownerLives()); | ||
| heartbeatTimeout = newHeartbeatTimeout; | ||
| } | ||
|
|
||
| function ownerLives() internal view returns (bool) { | ||
| return timeOfDeath == 0; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| import increaseTime from './helpers/increaseTime'; | ||
| import expectThrow from './helpers/expectThrow'; | ||
|
|
||
| const NULL_ADDRESS = '0x0000000000000000000000000000000000000000'; | ||
|
|
||
| const Heritable = artifacts.require('../contracts/ownership/Heritable.sol'); | ||
|
|
||
| contract('Heritable', function (accounts) { | ||
| let heritable; | ||
| let owner; | ||
|
|
||
| beforeEach(async function () { | ||
| heritable = await Heritable.new(4141); | ||
| owner = await heritable.owner(); | ||
| }); | ||
|
|
||
| it('should start off with an owner, but without heir', async function () { | ||
| const heir = await heritable.heir(); | ||
|
|
||
| assert.equal(typeof (owner), 'string'); | ||
| assert.equal(typeof (heir), 'string'); | ||
| assert.notStrictEqual( | ||
| owner, NULL_ADDRESS, | ||
| 'Owner shouldn\'t be the null address' | ||
| ); | ||
| assert.isTrue( | ||
| heir === NULL_ADDRESS, | ||
| 'Heir should be the null address' | ||
| ); | ||
| }); | ||
|
|
||
| it('only owner should set heir', async function () { | ||
| const newHeir = accounts[1]; | ||
| const someRandomAddress = accounts[2]; | ||
| assert.isTrue(owner !== someRandomAddress); | ||
|
|
||
| await heritable.setHeir(newHeir, { from: owner }); | ||
| await expectThrow(heritable.setHeir(newHeir, { from: someRandomAddress })); | ||
| }); | ||
|
|
||
| it('owner can remove heir', async function () { | ||
| const newHeir = accounts[1]; | ||
| await heritable.setHeir(newHeir, { from: owner }); | ||
| let heir = await heritable.heir(); | ||
|
|
||
| assert.notStrictEqual(heir, NULL_ADDRESS); | ||
| await heritable.removeHeir(); | ||
| heir = await heritable.heir(); | ||
| assert.isTrue(heir === NULL_ADDRESS); | ||
| }); | ||
|
|
||
| it('heir can claim ownership only if owner is dead and timeout was reached', async function () { | ||
| const heir = accounts[1]; | ||
| await heritable.setHeir(heir, { from: owner }); | ||
| await expectThrow(heritable.claimHeirOwnership({ from: heir })); | ||
|
|
||
| await heritable.proclaimDeath({ from: heir }); | ||
| await increaseTime(1); | ||
| await expectThrow(heritable.claimHeirOwnership({ from: heir })); | ||
|
|
||
| await increaseTime(4141); | ||
| await heritable.claimHeirOwnership({ from: heir }); | ||
| assert.isTrue(await heritable.heir() === heir); | ||
| }); | ||
|
|
||
| it('heir can\'t claim ownership if owner heartbeats', async function () { | ||
| const heir = accounts[1]; | ||
| await heritable.setHeir(heir, { from: owner }); | ||
|
|
||
| await heritable.proclaimDeath({ from: heir }); | ||
| await heritable.heartbeat({ from: owner }); | ||
| await expectThrow(heritable.claimHeirOwnership({ from: heir })); | ||
|
|
||
| await heritable.proclaimDeath({ from: heir }); | ||
| await increaseTime(4141); | ||
| await heritable.heartbeat({ from: owner }); | ||
| await expectThrow(heritable.claimHeirOwnership({ from: heir })); | ||
| }); | ||
|
|
||
| it('should log events appropriately', async function () { | ||
| const heir = accounts[1]; | ||
|
|
||
| const setHeirLogs = (await heritable.setHeir(heir, { from: owner })).logs; | ||
| const setHeirEvent = setHeirLogs.find(e => e.event === 'HeirChanged'); | ||
|
|
||
| assert.isTrue(setHeirEvent.args.owner === owner); | ||
| assert.isTrue(setHeirEvent.args.newHeir === heir); | ||
|
|
||
| const heartbeatLogs = (await heritable.heartbeat({ from: owner })).logs; | ||
| const heartbeatEvent = heartbeatLogs.find(e => e.event === 'OwnerHeartbeated'); | ||
|
|
||
| assert.isTrue(heartbeatEvent.args.owner === owner); | ||
|
|
||
| const proclaimDeathLogs = (await heritable.proclaimDeath({ from: heir })).logs; | ||
| const ownerDeadEvent = proclaimDeathLogs.find(e => e.event === 'OwnerProclaimedDead'); | ||
|
|
||
| assert.isTrue(ownerDeadEvent.args.owner === owner); | ||
| assert.isTrue(ownerDeadEvent.args.heir === heir); | ||
|
|
||
| await increaseTime(4141); | ||
| const claimHeirOwnershipLogs = (await heritable.claimHeirOwnership({ from: heir })).logs; | ||
| const ownershipTransferredEvent = claimHeirOwnershipLogs.find(e => e.event === 'OwnershipTransferred'); | ||
| const heirOwnershipClaimedEvent = claimHeirOwnershipLogs.find(e => e.event === 'HeirOwnershipClaimed'); | ||
|
|
||
| assert.isTrue(ownershipTransferredEvent.args.previousOwner === owner); | ||
| assert.isTrue(ownershipTransferredEvent.args.newOwner === heir); | ||
| assert.isTrue(heirOwnershipClaimedEvent.args.previousOwner === owner); | ||
| assert.isTrue(heirOwnershipClaimedEvent.args.newOwner === heir); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
|
|
||
| import expectThrow from './helpers/expectThrow'; | ||
|
|
||
| const SimpleSavingsWallet = artifacts.require('../contracts/examples/SimpleSavingsWallet.sol'); | ||
|
|
||
| contract('SimpleSavingsWallet', function (accounts) { | ||
| let savingsWallet; | ||
| let owner; | ||
|
|
||
| const paymentAmount = 4242; | ||
|
|
||
| beforeEach(async function () { | ||
| savingsWallet = await SimpleSavingsWallet.new(4141); | ||
| owner = await savingsWallet.owner(); | ||
| }); | ||
|
|
||
| it('should receive funds', async function () { | ||
| await web3.eth.sendTransaction({ from: owner, to: savingsWallet.address, value: paymentAmount }); | ||
| assert.isTrue((new web3.BigNumber(paymentAmount)).equals(web3.eth.getBalance(savingsWallet.address))); | ||
| }); | ||
|
|
||
| it('owner can send funds', async function () { | ||
| // Receive payment so we have some money to spend. | ||
| await web3.eth.sendTransaction({ from: accounts[9], to: savingsWallet.address, value: 1000000 }); | ||
| await expectThrow(savingsWallet.sendTo(0, paymentAmount, { from: owner })); | ||
| await expectThrow(savingsWallet.sendTo(savingsWallet.address, paymentAmount, { from: owner })); | ||
| await expectThrow(savingsWallet.sendTo(accounts[1], 0, { from: owner })); | ||
|
|
||
| const balance = web3.eth.getBalance(accounts[1]); | ||
| await savingsWallet.sendTo(accounts[1], paymentAmount, { from: owner }); | ||
| assert.isTrue(balance.plus(paymentAmount).equals(web3.eth.getBalance(accounts[1]))); | ||
| }); | ||
| }); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we provide a setter, I would go with a
privatevisibility for theheirand providing a getter as well. This will ensure that inheriting contracts can not modify theheirin another way than the one proposed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we may want the same thing for
heartbeatTimeoutandtimeOfDeathThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, but this is not something we're implementing in general in OZ. Eg. Ownable has:
address public ownerThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually we are caring about this from now on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great. I won't be changing it in this PR tho.
I will create an issue instead and ping @azavalla