Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions contracts/chequebook/contract/chequebook.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import "mortal";
pragma solidity ^0.4.18;

import "https://github.com/ethereum/solidity/std/mortal.sol";

/// @title Chequebook for Ethereum micropayments
/// @author Daniel A. Nagy <daniel@ethdev.com>
/// @author Daniel A. Nagy <daniel@ethereum.org>
contract chequebook is mortal {
// Cumulative paid amount in wei to each beneficiary
mapping (address => uint256) public sent;
Expand All @@ -21,26 +23,23 @@ contract chequebook is mortal {
uint8 sig_v, bytes32 sig_r, bytes32 sig_s) {
// Check if the cheque is old.
// Only cheques that are more recent than the last cashed one are considered.
if(amount <= sent[beneficiary]) return;
require(amount > sent[beneficiary]);
// Check the digital signature of the cheque.
bytes32 hash = sha3(address(this), beneficiary, amount);
if(owner != ecrecover(hash, sig_v, sig_r, sig_s)) return;
bytes32 hash = keccak256(address(this), beneficiary, amount);
require(owner == ecrecover(hash, sig_v, sig_r, sig_s));
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two require changes change behaviour: before if the owner/amount didn't match, it resulted in a successful execution without doing anything; now it results in a failed execution using the revert opcode (remainder gas is not consumed).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that good or bad @nagydani ?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, it is okay. The actual effect is the same: nothing changes in the state and remaining gas is not consumed.

// Attempt sending the difference between the cumulative amount on the cheque
// and the cumulative amount on the last cashed cheque to beneficiary.
uint256 diff = amount - sent[beneficiary];
if (diff <= this.balance) {
// update the cumulative amount before sending
sent[beneficiary] = amount;
if (!beneficiary.send(diff)) {
// Upon failure to execute send, revert everything
throw;
}
beneficiary.transfer(diff);
} else {
// Upon failure, punish owner for writing a bounced cheque.
// owner.sendToDebtorsPrison();
Overdraft(owner);
// Compensate beneficiary.
suicide(beneficiary);
selfdestruct(beneficiary);
}
}
}