From f7816493553c7fc5cbb2287e01971e5da85a7ac0 Mon Sep 17 00:00:00 2001 From: Griff Green Date: Tue, 26 Sep 2017 10:57:09 +0200 Subject: [PATCH 1/5] added comments --- contracts/Vault.sol | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/contracts/Vault.sol b/contracts/Vault.sol index 260518c..63e971b 100644 --- a/contracts/Vault.sol +++ b/contracts/Vault.sol @@ -1,37 +1,54 @@ pragma solidity ^0.4.11; +/// @title Vault +/// @author Jordi Baylina +/// @notice This contract holds ether securely for liquid pledging systems. For +/// this iteration the funds will come straight from the Giveth Multisig as a +/// safety precaution, but once fully tested and optimized this contract will +/// be a safe place to store funds equipped with optional variable time delays +/// to allow for an optional escape hatch to be implemented import "./Owned.sol"; +/// @dev This is declares a few functions from `LiquidPledging` so that the +/// `Vault` contract can interface with the `LiquidPledging` contract contract LiquidPledging { function confirmPayment(uint64 idNote, uint amount); function cancelPayment(uint64 idNote, uint amount); } + +/// @dev `Vault` is a higher level contract built off of the `Owned` +/// contract that holds funds for the liquid pledging system. contract Vault is Owned { - LiquidPledging public liquidPledging; - bool public autoPay; + LiquidPledging public liquidPledging; // liquidPledging contract's address + bool public autoPay; // if false, payments will take 2 txs to be completed enum PaymentState { - Pending, - Paid, - Canceled + Pending, // means the payment is awaiting confirmation + Paid, // means the payment has been sent + Canceled // means the payment will never be sent } - + /// @dev `Payment` is a public structure that describes the details of + /// each payment the `ref` param makes it easy to track the movements of + /// funds transparently by its connection to other `Payment` structs struct Payment { - PaymentState state; - bytes32 ref; - address dest; - uint amount; + PaymentState state; // + bytes32 ref; // a hash that references details from other contracts + address dest; // recipient of the ETH + uint amount; // amount of ETH (in wei) to be sent } + // @dev An array that contains all the payments for this Vault Payment[] public payments; + // @dev `liquidPledging` is the only address that can call a function with + /// this modifier modifier onlyLiquidPledging() { require(msg.sender == address(liquidPledging)); _; } - + /// @dev USED FOR TESTING??? function VaultMock() { } @@ -74,7 +91,7 @@ contract Vault is Owned { require(p.state == PaymentState.Pending); p.state = PaymentState.Paid; - p.dest.transfer(p.amount); + p.dest.transfer(p.amount); // only ETH denominated in wei liquidPledging.confirmPayment(uint64(p.ref), p.amount); From aca20feb5319ae8de820d98ff292b0b059e965d4 Mon Sep 17 00:00:00 2001 From: perissology Date: Tue, 26 Sep 2017 12:06:45 -0700 Subject: [PATCH 2/5] return plugin in getNoteManager --- contracts/LiquidPledgingBase.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contracts/LiquidPledgingBase.sol b/contracts/LiquidPledgingBase.sol index a896637..d8d20a2 100644 --- a/contracts/LiquidPledgingBase.sol +++ b/contracts/LiquidPledgingBase.sol @@ -232,7 +232,8 @@ contract LiquidPledgingBase { string name, uint64 commitTime, uint64 parentProject, - bool canceled) + bool canceled, + address plugin) { NoteManager storage m = findManager(idManager); managerType = m.managerType; @@ -241,6 +242,7 @@ contract LiquidPledgingBase { commitTime = m.commitTime; parentProject = m.parentProject; canceled = m.canceled; + plugin = m.plugin; } //////// From e68546451ebf4a7b1b66d34ff2619b75e2f1b970 Mon Sep 17 00:00:00 2001 From: perissology Date: Tue, 26 Sep 2017 12:26:26 -0700 Subject: [PATCH 3/5] explict type conversion for Plugin --- contracts/LiquidPledgingBase.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/LiquidPledgingBase.sol b/contracts/LiquidPledgingBase.sol index d8d20a2..7124ebe 100644 --- a/contracts/LiquidPledgingBase.sol +++ b/contracts/LiquidPledgingBase.sol @@ -242,7 +242,7 @@ contract LiquidPledgingBase { commitTime = m.commitTime; parentProject = m.parentProject; canceled = m.canceled; - plugin = m.plugin; + plugin = address(m.plugin); } //////// From da9a2766470f22d2b6db24954f5a76be90b76fad Mon Sep 17 00:00:00 2001 From: Griff Green Date: Fri, 29 Sep 2017 11:06:11 +0200 Subject: [PATCH 4/5] Added comments and asserted 80 char per line --- contracts/ILiquidPledgingPlugin.sol | 37 +++++++++++++++++++---------- 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/contracts/ILiquidPledgingPlugin.sol b/contracts/ILiquidPledgingPlugin.sol index f217cfe..a9d7f26 100644 --- a/contracts/ILiquidPledgingPlugin.sol +++ b/contracts/ILiquidPledgingPlugin.sol @@ -1,19 +1,32 @@ pragma solidity ^0.4.11; contract ILiquidPledgingPlugin { - - /// @param context In which context it is affected. - /// 0 -> owner from - /// 1 -> First delegate from - /// 2 -> Second delegate from + /// @notice Plugins are used (much like web hooks) to initiate an action + /// upon any donation, delegation, or transfer; this is an optional feature + /// and allows for extreme customization of the contract + /// @param context The situation that is triggering the plugin: + /// 0 -> Plugin for the owner transferring pledge to another party + /// 1 -> Plugin for the first delegate transferring pledge to another party + /// 2 -> Plugin for the second delegate transferring pledge to another party /// ... - /// 255 -> proposedProject from + /// 255 -> Plugin for the proposedProject transferring pledge to another party /// - /// 256 -> owner to - /// 257 -> First delegate to - /// 258 -> Second delegate to + /// 256 -> Plugin for the owner receiving pledge to another party + /// 257 -> Plugin for the first delegate receiving pledge to another party + /// 258 -> Plugin for the second delegate receiving pledge to another party /// ... - /// 511 -> proposedProject to - function beforeTransfer(uint64 noteManager, uint64 noteFrom, uint64 noteTo, uint64 context, uint amount) returns (uint maxAllowed); - function afterTransfer(uint64 noteManager, uint64 noteFrom, uint64 noteTo, uint64 context, uint amount); + /// 511 -> Plugin for the proposedProject receiving pledge to another party + function beforeTransfer( + uint64 noteManager, + uint64 noteFrom, + uint64 noteTo, + uint64 context, + uint amount + ) returns (uint maxAllowed); + function afterTransfer( + uint64 noteManager, + uint64 noteFrom, + uint64 noteTo, + uint64 context, + uint amount); } From 885678c0787ef91bb3d495fedb6d17e430dedcf8 Mon Sep 17 00:00:00 2001 From: Griff Green Date: Fri, 29 Sep 2017 12:11:13 +0200 Subject: [PATCH 5/5] Added comments and a few other small things --- contracts/LiquidPledgingBase.sol | 78 ++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 34 deletions(-) diff --git a/contracts/LiquidPledgingBase.sol b/contracts/LiquidPledgingBase.sol index a896637..bf80cf5 100644 --- a/contracts/LiquidPledgingBase.sol +++ b/contracts/LiquidPledgingBase.sol @@ -2,43 +2,46 @@ pragma solidity ^0.4.11; import "./ILiquidPledgingPlugin.sol"; +/// @dev This is declares a few functions from `Vault` so that the +/// `LiquidPledgingBase` contract can interface with the `Vault` contract contract Vault { function authorizePayment(bytes32 _ref, address _dest, uint _amount); function () payable; } contract LiquidPledgingBase { - + // Limits inserted to prevent large loops that could prevent canceling uint constant MAX_DELEGATES = 20; uint constant MAX_SUBPROJECT_LEVEL = 20; uint constant MAX_INTERPROJECT_LEVEL = 20; - enum NoteManagerType { Donor, Delegate, Project }// todo change name - enum PaymentState { NotPaid, Paying, Paid } + enum NoteManagerType { Donor, Delegate, Project } // todo change name Donor Project + enum PaymentState { NotPaid, Paying, Paid } // TODO name change NotPaid - // This struct defines the details of each the NoteManager, these NoteManagers can create - struct NoteManager {// change manager - NoteManagerType managerType; - address addr; - string name; - uint64 commitTime; // Only used in donors and projects, its the precommitment time - uint64 parentProject; // Only for projects - bool canceled; // Only for project - ILiquidPledgingPlugin plugin; // Handler that is called when one call is affected. + /// @dev This struct defines the details of each the NoteManager, these + /// NoteManagers can own notes and act as delegates + struct NoteManager { // TODO name change NoteManager + NoteManagerType managerType; // Giver, Delegate or Campaign + address addr; // account or contract address for admin + string name; + uint64 commitTime; // In seconds, used for Givers' & Delegates' vetos + uint64 parentProject; // Only for campaigns + bool canceled; //Always false except for canceled campaigns + ILiquidPledgingPlugin plugin; // if the plugin is 0x0 then nothing happens if its a contract address than that smart contract is called via the milestone contract } struct Note { uint amount; - uint64 owner; - uint64[] delegationChain; //index numbers!!!!! + uint64 owner; //NoteManager + uint64[] delegationChain; // list of index numbers uint64 proposedProject; // TODO change the name only used for when delegates are precommiting to a project - uint64 commitTime; // At what time the upcoming time will become an owner. + uint64 commitTime; // When the proposedProject will become the owner uint64 oldNote; // this points to the Note[] index that the Note was derived from PaymentState paymentState; } Note[] notes; - NoteManager[] managers; // the list of all the note managers 0 is reserved for no manager + NoteManager[] managers; //The list of noteManagers 0 means there is no manager Vault public vault; // this mapping allows you to search for a specific note's index number by the hash of that note @@ -59,6 +62,8 @@ contract LiquidPledgingBase { // Constructor ////// + /// @notice The Constructor creates the `LiquidPledgingBase` on the blockchain + /// @param _vault Where the ETH is stored that the pledges represent function LiquidPledgingBase(address _vault) { managers.length = 1; // we reserve the 0 manager notes.length = 1; // we reserve the 0 note @@ -70,7 +75,9 @@ contract LiquidPledgingBase { // Managers functions ////// - function addDonor(string name, uint64 commitTime, ILiquidPledgingPlugin plugin) returns (uint64 idDonor) {//Todo return idManager + /// @notice Creates a donor. + function addDonor(string name, uint64 commitTime, ILiquidPledgingPlugin plugin + ) returns (uint64 idDonor) {//Todo return idManager idDonor = uint64(managers.length); @@ -88,6 +95,7 @@ contract LiquidPledgingBase { event DonorAdded(uint64 indexed idDonor); + ///@notice Changes the address, name or commitTime associated with a specific donor function updateDonor( uint64 idDonor, address newAddr, @@ -95,8 +103,8 @@ contract LiquidPledgingBase { uint64 newCommitTime) { NoteManager storage donor = findManager(idDonor); - require(donor.managerType == NoteManagerType.Donor); - require(donor.addr == msg.sender); + require(donor.managerType == NoteManagerType.Donor);//Must be a Giver + require(donor.addr == msg.sender);//current addr had to originate this tx donor.addr = newAddr; donor.name = newName; donor.commitTime = newCommitTime; @@ -105,6 +113,7 @@ contract LiquidPledgingBase { event DonorUpdated(uint64 indexed idDonor); + /// @notice Creates a new Delegate function addDelegate(string name, uint64 commitTime, ILiquidPledgingPlugin plugin) returns (uint64 idDelegate) { //TODO return index number idDelegate = uint64(managers.length); @@ -123,6 +132,7 @@ contract LiquidPledgingBase { event DeegateAdded(uint64 indexed idDelegate); + ///@notice Changes the address, name or commitTime associated with a specific delegate function updateDelegate( uint64 idDelegate, address newAddr, @@ -139,6 +149,7 @@ contract LiquidPledgingBase { event DelegateUpdated(uint64 indexed idDelegate); + /// @notice Creates a new Campaign function addProject(string name, address projectManager, uint64 parentProject, uint64 commitTime, ILiquidPledgingPlugin plugin) returns (uint64 idProject) { if (parentProject != 0) { NoteManager storage pm = findManager(parentProject); @@ -164,6 +175,7 @@ contract LiquidPledgingBase { event ProjectAdded(uint64 indexed idProject); + ///@notice Changes the address, name or commitTime associated with a specific Campaign function updateProject( uint64 idProject, address newAddr, @@ -186,11 +198,11 @@ contract LiquidPledgingBase { // Public constant functions ////////// - + /// @notice Public constant that states how many notes are in the system function numberOfNotes() constant returns (uint) { return notes.length - 1; } - + /// @notice Public constant that states the details of the specified Note function getNote(uint64 idNote) constant returns( uint amount, uint64 owner, @@ -209,7 +221,8 @@ contract LiquidPledgingBase { oldNote = n.oldNote; paymentState = n.paymentState; } - // This is to return the delegates one by one, because you can not return an array + /// @notice Public constant that states the delegates one by one, because + /// an array cannot be returned function getNoteDelegate(uint64 idNote, uint idxDelegate) constant returns( uint64 idDelegate, address addr, @@ -221,11 +234,11 @@ contract LiquidPledgingBase { addr = delegate.addr; name = delegate.name; } - + /// @notice Public constant that states the number of admins in the system function numberOfNoteManagers() constant returns(uint) { return managers.length - 1; } - + /// @notice Public constant that states the details of the specified admin function getNoteManager(uint64 idManager) constant returns ( NoteManagerType managerType, address addr, @@ -247,9 +260,10 @@ contract LiquidPledgingBase { // Private methods /////// - // All notes exist... but if the note hasn't been created in this system yet then it wouldn't - // be in the hash array hNoteddx[] - // this function creates a balloon if one is not created already... this ballon has 0 for the amount + /// @notice All notes technically exist... but if the note hasn't been + /// created in this system yet then it wouldn't be in the hash array + /// hNoteddx[]; this creates a Pledge with and amount of 0 if one is not + /// created already... function findNote( uint64 owner, uint64[] delegationChain, @@ -282,7 +296,7 @@ contract LiquidPledgingBase { uint64 constant NOTFOUND = 0xFFFFFFFFFFFFFFFF; // helper function that searches the delegationChain fro a specific delegate and - // level of delegation returns their idx in the delegation cahin which reflect their level of authority + // level of delegation returns their idx in the delegation chain which reflect their level of authority function getDelegateIdx(Note n, uint64 idDelegate) internal returns(uint64) { for (uint i=0; i