Conversation
| } | ||
|
|
||
| function validate() public { | ||
| jurisdiction.addAttribute(msg.sender, "VALID", 1); |
There was a problem hiding this comment.
I found this weird, because the Registry doesn't have addAttribute, just hasAttribute.
Maybe, the idea is to have a checker that just checks hasAttribute, and a validator that calls addAttribute.
Or maybe we should include addAttribute to Registry, so it's nicer to do both on the same validator.
There was a problem hiding this comment.
Registry seems to be the read-only interface of Jurisdiction. 🤷♂️
It looks good to me like this.
There was a problem hiding this comment.
What's weird is that now the validator has a variable called registry and another called jurisdiction, and they point to the same instance.
There was a problem hiding this comment.
Ah, right. I don't see a way out of this. We can't cast the registry variable safely because we don't know if TransactionChecker ever changes it to be something different.
| returns (bool) | ||
| { | ||
| require(jurisdiction.hasAttribute(_to, "VALID")); | ||
| require(validator.transferAllowed(msg.sender, _to, _amount)); |
There was a problem hiding this comment.
This is not clear in the TPL spec, but I think we should be checking transferAllowed(0x0, _to, _amount), i.e. from = 0x0 which is usually how minting is thought of.
| } | ||
|
|
||
| function validate() public { | ||
| jurisdiction.addAttribute(msg.sender, "VALID", 1); |
There was a problem hiding this comment.
Registry seems to be the read-only interface of Jurisdiction. 🤷♂️
It looks good to me like this.
2 participants
Fixes #15