ExternalLocking4Rep : add registrar #562
Conversation
| string public getBalanceFuncSignature; | ||
|
|
||
| // locker -> bool | ||
| mapping(address => bool) public externalLockers; |
There was a problem hiding this comment.
I think the code would be more clear if this were called "claimers".
There was a problem hiding this comment.
Even better: "claimants".
| * @return claimId | ||
| */ | ||
| function lock() public returns(bytes32) { | ||
| function claim(address _beneficiary) public returns(bytes32) { |
There was a problem hiding this comment.
Can we call "_beneficiary" "_claimer"?
There was a problem hiding this comment.
But it is not the claimer, as one can claim for someone else, who was registered.
There was a problem hiding this comment.
@leviadam I view this as a person having "authorized" anyone else to claim the tokens for them. The word "claimant" would be better here than "claimer".
So I might rename the function "Register" to something else. A legal term is "Release" (to release the exclusive right to do something). Maybe "Authorize", though one is not authorizing any particular account to do the claim.
Mainly I think we're being inconsistent inside the contract with the sense of "locking" and "claiming", and that is the primary source of confusion.
Further, frankly I'm not wild about "claim" either. My understanding of the Magnolia contract is that what this "claim" method is doing is simply placing the claimant's tokens into a mapping from which the tokens can eventually be "unlocked" (placed in another mapping), then withdrawn (placed in another mapping) that finally allows the owner of the tokens to have control over the tokens.
So what "claim" is doing is not in fact claiming anything, as the Magnolia contract currently stands (in Kovan, at 0x4eDc383aDEa781762b74E7082C03F423523e61Bb).
Unless the contract is changing, "Lock" seems appropriate here.
There was a problem hiding this comment.
msg.sender in this case is claiming on behalf of the claimant (here _beneficiary), who "registered" (poor word IMHO) to allow someone else (here msg.sender) to "claim" on their behalf.
msg.sender is not here claiming anything, rather is executing a claim an behalf of someone else.
|
|
||
| contract ExternalLocking4Reputation is Locking4Reputation, Ownable { | ||
|
|
||
| event Register(address indexed _beneficiary); |
There was a problem hiding this comment.
I think I would refer to this everywhere as "Authorizing" rather than "Registering"
| * @return claimId | ||
| */ | ||
| function lock() public returns(bytes32) { | ||
| function claim(address _beneficiary) public returns(bytes32) { |
There was a problem hiding this comment.
@leviadam I view this as a person having "authorized" anyone else to claim the tokens for them. The word "claimant" would be better here than "claimer".
So I might rename the function "Register" to something else. A legal term is "Release" (to release the exclusive right to do something). Maybe "Authorize", though one is not authorizing any particular account to do the claim.
Mainly I think we're being inconsistent inside the contract with the sense of "locking" and "claiming", and that is the primary source of confusion.
Further, frankly I'm not wild about "claim" either. My understanding of the Magnolia contract is that what this "claim" method is doing is simply placing the claimant's tokens into a mapping from which the tokens can eventually be "unlocked" (placed in another mapping), then withdrawn (placed in another mapping) that finally allows the owner of the tokens to have control over the tokens.
So what "claim" is doing is not in fact claiming anything, as the Magnolia contract currently stands (in Kovan, at 0x4eDc383aDEa781762b74E7082C03F423523e61Bb).
Unless the contract is changing, "Lock" seems appropriate here.
rename lock to claim .
claiming can be done for behalf of someone else only if it is register .
add register function.