-
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
Changes from 23 commits
4fe2157
2a560ad
b709206
a613cd0
a8afb20
d808e49
80ae074
560a855
4332135
c7807c5
5716492
e596046
82c8512
46736da
c70ee93
fe712c6
52b6181
51c2c50
5ea9bd4
3009553
22c1403
9d005b4
c01203b
a5ea0af
c8c0f21
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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); | ||
| } | ||
| } |
| 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()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we want a heartbeat to be greater than 0?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean? Do you mean that we should check for |
||
| heartbeatTimeout = newHeartbeatTimeout; | ||
| } | ||
|
|
||
| function ownerLives() internal view returns (bool) { | ||
| return timeOfDeath == 0; | ||
| } | ||
| } | ||
| 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); | ||
| }); | ||
| }); |
| 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]))); | ||
| }); | ||
| }); |
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