Address validation (#156)

* Fix create step query

* Add basic regex validation for addresses to frontend.

* Check with zcash node if address is valid before final proposal submission.

* tsc

* Mock requests where needed. Come up with a function that mocks all blockchain requests.

* Remove print

backend/grant/proposal/models.py

@@ -212,6 +212,11 @@ class Proposal(db.Model):
             if not hasattr(self, field):
                 raise ValidationException("Proposal must have a {}".format(field))
 
+        # Check with node that the address is kosher
+        res = blockchain_get('/validate/address', {'address': self.payout_address})
+        if not res['valid']:
+            raise ValidationException("Payout address is not a valid Zcash address")
+
         # Then run through regular validation
         Proposal.validate(vars(self))
 

backend/tests/admin/test_api.py

@@ -3,7 +3,7 @@ from grant.utils.admin import generate_admin_password_hash
 from mock import patch
 
 from ..config import BaseProposalCreatorConfig
-from ..mocks import mock_request
+from ..test_data import mock_blockchain_api_requests
 
 
 plaintext_mock_password = "p4ssw0rd"
@@ -96,7 +96,8 @@ class TestAdminAPI(BaseProposalCreatorConfig):
         resp = self.app.put(f"/api/v1/admin/proposals/{self.proposal.id}", data={"contributionMatching": 2})
         self.assert400(resp)
 
-    def test_approve_proposal(self):
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
+    def test_approve_proposal(self, mock_get):
         self.login_admin()
 
         # proposal needs to be PENDING
@@ -110,7 +111,8 @@ class TestAdminAPI(BaseProposalCreatorConfig):
         self.assert200(resp)
         self.assertEqual(resp.json["status"], ProposalStatus.APPROVED)
 
-    def test_reject_proposal(self):
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
+    def test_reject_proposal(self, mock_get):
         self.login_admin()
 
         # proposal needs to be PENDING

backend/tests/config.py

@@ -9,7 +9,7 @@ from grant.user.models import User, SocialMedia, db, Avatar
 from grant.settings import PROPOSAL_STAKING_AMOUNT
 from grant.utils.enums import ProposalStatus
 
-from .test_data import test_user, test_other_user, test_proposal, mock_contribution_addresses
+from .test_data import test_user, test_other_user, test_proposal, mock_blockchain_api_requests
 
 
 class BaseTestConfig(TestCase):
@@ -148,7 +148,7 @@ class BaseProposalCreatorConfig(BaseUserConfig):
         proposal_reminder = ProposalReminder(self.proposal.id)
         proposal_reminder.make_task()
 
-    @patch('requests.get', side_effect=mock_contribution_addresses)
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
     def stake_proposal(self, mock_get):
         # 1. submit
         self.proposal.submit_for_approval()

backend/tests/proposal/test_api.py

@@ -6,7 +6,15 @@ from grant.proposal.models import Proposal
 from grant.utils.enums import ProposalStatus
 
 from ..config import BaseProposalCreatorConfig
-from ..test_data import test_proposal, mock_contribution_addresses
+from ..test_data import test_proposal, mock_blockchain_api_requests, mock_invalid_address
+
+
+# Used when a test mocks request.get in multiple ways
+def mock_contribution_addresses_and_valid_address(path):
+    if path == '/contribution/addresses':
+        return mock_valid_address
+    else:
+        return mock_contribution_addresses
 
 
 class TestProposalAPI(BaseProposalCreatorConfig):
@@ -68,29 +76,39 @@ class TestProposalAPI(BaseProposalCreatorConfig):
         self.assert404(resp)
 
     # /submit_for_approval
-    def test_proposal_draft_submit_for_approval(self):
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
+    def test_proposal_draft_submit_for_approval(self, mock_get):
         self.login_default_user()
         resp = self.app.put("/api/v1/proposals/{}/submit_for_approval".format(self.proposal.id))
         self.assert200(resp)
         self.assertEqual(resp.json['status'], ProposalStatus.STAKING)
 
-    def test_no_auth_proposal_draft_submit_for_approval(self):
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
+    def test_no_auth_proposal_draft_submit_for_approval(self, mock_get):
         resp = self.app.put("/api/v1/proposals/{}/submit_for_approval".format(self.proposal.id))
         self.assert401(resp)
 
-    def test_invalid_proposal_draft_submit_for_approval(self):
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
+    def test_invalid_proposal_draft_submit_for_approval(self, mock_get):
         self.login_default_user()
         resp = self.app.put("/api/v1/proposals/12345/submit_for_approval")
         self.assert404(resp)
 
-    def test_invalid_status_proposal_draft_submit_for_approval(self):
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
+    def test_invalid_status_proposal_draft_submit_for_approval(self, mock_get):
         self.login_default_user()
         self.proposal.status = ProposalStatus.PENDING  # should be ProposalStatus.DRAFT
         resp = self.app.put("/api/v1/proposals/{}/submit_for_approval".format(self.proposal.id))
         self.assert400(resp)
 
+    @patch('requests.get', side_effect=mock_invalid_address)
+    def test_invalid_address_proposal_draft_submit_for_approval(self, mock_get):
+        self.login_default_user()
+        resp = self.app.put("/api/v1/proposals/{}/submit_for_approval".format(self.proposal.id))
+        self.assert400(resp)
+
     # /stake
-    @patch('requests.get', side_effect=mock_contribution_addresses)
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
     def test_proposal_stake(self, mock_get):
         self.login_default_user()
         self.proposal.status = ProposalStatus.STAKING
@@ -99,14 +117,14 @@ class TestProposalAPI(BaseProposalCreatorConfig):
         self.assert200(resp)
         self.assertEquals(resp.json['amount'], str(PROPOSAL_STAKING_AMOUNT))
 
-    @patch('requests.get', side_effect=mock_contribution_addresses)
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
     def test_proposal_stake_no_auth(self, mock_get):
         self.proposal.status = ProposalStatus.STAKING
         resp = self.app.get(f"/api/v1/proposals/{self.proposal.id}/stake")
         print(resp)
         self.assert401(resp)
 
-    @patch('requests.get', side_effect=mock_contribution_addresses)
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
     def test_proposal_stake_bad_status(self, mock_get):
         self.login_default_user()
         self.proposal.status = ProposalStatus.PENDING  # should be staking
@@ -114,7 +132,7 @@ class TestProposalAPI(BaseProposalCreatorConfig):
         print(resp)
         self.assert400(resp)
 
-    @patch('requests.get', side_effect=mock_contribution_addresses)
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
     def test_proposal_stake_funded(self, mock_get):
         self.login_default_user()
         # fake stake contribution with confirmation
@@ -124,29 +142,34 @@ class TestProposalAPI(BaseProposalCreatorConfig):
         self.assert404(resp)
 
     # /publish
-    def test_publish_proposal_approved(self):
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
+    def test_publish_proposal_approved(self, mock_get):
         self.login_default_user()
         # proposal needs to be APPROVED
         self.proposal.status = ProposalStatus.APPROVED
         resp = self.app.put("/api/v1/proposals/{}/publish".format(self.proposal.id))
         self.assert200(resp)
 
-    def test_no_auth_publish_proposal(self):
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
+    def test_no_auth_publish_proposal(self, mock_get):
         resp = self.app.put("/api/v1/proposals/{}/publish".format(self.proposal.id))
         self.assert401(resp)
 
-    def test_invalid_proposal_publish_proposal(self):
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
+    def test_invalid_proposal_publish_proposal(self, mock_get):
         self.login_default_user()
         resp = self.app.put("/api/v1/proposals/12345/publish")
         self.assert404(resp)
 
-    def test_invalid_status_proposal_publish_proposal(self):
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
+    def test_invalid_status_proposal_publish_proposal(self, mock_get):
         self.login_default_user()
         self.proposal.status = ProposalStatus.PENDING  # should be ProposalStatus.APPROVED
         resp = self.app.put("/api/v1/proposals/{}/publish".format(self.proposal.id))
         self.assert400(resp)
 
-    def test_not_verified_email_address_publish_proposal(self):
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
+    def test_not_verified_email_address_publish_proposal(self, mock_get):
         self.login_default_user()
         self.mark_user_not_verified()
         self.proposal.status = "DRAFT"

backend/tests/proposal/test_contribution_api.py

@@ -4,18 +4,12 @@ from mock import patch
 from grant.proposal.models import Proposal
 from grant.utils.enums import ProposalStatus
 from ..config import BaseProposalCreatorConfig
-from ..test_data import test_proposal
+from ..test_data import test_proposal, mock_blockchain_api_requests
 from ..mocks import mock_request
 
-mock_contribution_addresses = mock_request({
-    'transparent': 't123',
-    'sprout': 'z123',
-    'memo': '123',
-})
-
 
 class TestProposalContributionAPI(BaseProposalCreatorConfig):
-    @patch('requests.get', side_effect=mock_contribution_addresses)
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
     def test_create_proposal_contribution(self, mock_blockchain_get):
         self.login_default_user()
 
@@ -31,7 +25,7 @@ class TestProposalContributionAPI(BaseProposalCreatorConfig):
 
         self.assertStatus(post_res, 201)
 
-    @patch('requests.get', side_effect=mock_contribution_addresses)
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
     def test_create_duplicate_contribution(self, mock_blockchain_get):
         self.login_default_user()
 
@@ -55,7 +49,7 @@ class TestProposalContributionAPI(BaseProposalCreatorConfig):
         self.assert200(dupe_res)
         self.assertEqual(dupe_res.json['id'], post_res.json['id'])
 
-    @patch('requests.get', side_effect=mock_contribution_addresses)
+    @patch('requests.get', side_effect=mock_blockchain_api_requests)
     def test_get_proposal_contribution(self, mock_blockchain_get):
         self.login_default_user()
 

backend/tests/test_data.py

@@ -64,3 +64,19 @@ mock_contribution_addresses = mock_request({
     'sprout': 'z123',
     'memo': '123',
 })
+
+mock_valid_address = mock_request({
+    'valid': True,
+})
+
+mock_invalid_address = mock_request({
+    'valid': False,
+})
+
+
+def mock_blockchain_api_requests(path, **kwargs):
+    if '/contribution/addresses' in path:
+        return mock_contribution_addresses()
+    if '/validate/address' in path:
+        return mock_valid_address()
+    raise Exception('No mock data defined for path {}'.format(path))

blockchain/src/node.ts

@@ -102,6 +102,12 @@ export interface DisclosedPayment {
   message?: string;
 }
 
+// Actually comes back with a bunch of args, but this is all we need
+export interface ValidationResponse {
+  isvalid: boolean;
+}
+
+
 // TODO: Type all methods with signatures from
 // https://github.com/zcash/zcash/blob/master/doc/payment-api.md
 interface ZCashNode {
@@ -113,6 +119,7 @@ interface ZCashNode {
     (numberOrHash: string | number, verbosity: 0): Promise<string>;
   }
   gettransaction: (txid: string) => Promise<Transaction>;
+  validateaddress: (address: string) => Promise<ValidationResponse>;
   z_getbalance: (address: string, minConf?: number) => Promise<number>;
   z_getnewaddress: (type?: 'sprout' | 'sapling') => Promise<string>;
   z_listaddresses: () => Promise<string[]>;
@@ -120,6 +127,7 @@ interface ZCashNode {
   z_importviewingkey: (key: string, rescan?: 'yes' | 'no' | 'whenkeyisnew', startHeight?: number) => Promise<void>;
   z_exportviewingkey: (zaddr: string) => Promise<string>;
   z_validatepaymentdisclosure: (disclosure: string) => Promise<DisclosedPayment>;
+  z_validateaddress: (address: string) => Promise<ValidationResponse>;
 }
 
 export const rpcOptions = {

blockchain/src/server/index.ts

@@ -101,6 +101,19 @@ app.post('/contribution/disclosure', async (req, res) => {
   }
 });
 
+app.get('/validate/address', async (req, res) => {
+  const { address } = req.query;
+  const [tRes, zRes] = await Promise.all([
+    node.validateaddress(address as string),
+    node.z_validateaddress(address as string),
+  ]);
+  return res.json({
+    data: {
+      valid: tRes.isvalid || zRes.isvalid,
+    },
+  });
+});
+
 // Error handler after all routes to catch thrown exceptions
 app.use(errorHandlerMiddleware);
 

frontend/client/components/CreateFlow/Final.tsx

@@ -10,6 +10,10 @@ import './Final.less';
 import PaymentInfo from 'components/ContributionModal/PaymentInfo';
 import { ContributionWithAddresses } from 'types';
 
+interface OwnProps {
+  goBack(): void;
+}
+
 interface StateProps {
   form: AppState['create']['form'];
   submittedProposal: AppState['create']['submittedProposal'];
@@ -20,7 +24,7 @@ interface DispatchProps {
   submitProposal: typeof createActions['submitProposal'];
 }
 
-type Props = StateProps & DispatchProps;
+type Props = OwnProps & StateProps & DispatchProps;
 
 const STATE = {
   contribution: null as null | ContributionWithAddresses,
@@ -44,7 +48,7 @@ class CreateFinal extends React.Component<Props, State> {
   }
 
   render() {
-    const { submittedProposal, submitError } = this.props;
+    const { submittedProposal, submitError, goBack } = this.props;
     const { contribution } = this.state;
 
     const ready = submittedProposal && (submittedProposal.isStaked || contribution);
@@ -60,7 +64,7 @@ class CreateFinal extends React.Component<Props, State> {
               <b>Something went wrong during creation</b>
             </h3>
             <h5>{submitError}</h5>
-            <a onClick={this.submit}>Click here</a> to try again.
+            <a onClick={goBack}>Click here</a> to go back to the form and try again.
           </div>
         </div>
       );
@@ -138,7 +142,7 @@ class CreateFinal extends React.Component<Props, State> {
   };
 }
 
-export default connect<StateProps, DispatchProps, {}, AppState>(
+export default connect<StateProps, DispatchProps, OwnProps, AppState>(
   (state: AppState) => ({
     form: state.create.form,
     submittedProposal: state.create.submittedProposal,

frontend/client/components/CreateFlow/example.ts

@@ -1,15 +1,19 @@
 import { PROPOSAL_CATEGORY } from 'api/constants';
 import { ProposalDraft } from 'types';
 
-const createExampleProposal = (payoutAddress: string): Partial<ProposalDraft> => {
+const createExampleProposal = (): Partial<ProposalDraft> => {
+  const cats = Object.keys(PROPOSAL_CATEGORY);
+  const category = cats[Math.floor(Math.random() * cats.length)] as PROPOSAL_CATEGORY;
   return {
     title: 'Grant.io T-Shirts',
     brief: "The most stylish wear, sporting your favorite brand's logo",
-    category: PROPOSAL_CATEGORY.COMMUNITY,
+    category,
     content:
       '![](https://i.imgur.com/aQagS0D.png)\n\nWe all know it, Grant.io is the bee\'s knees. But wouldn\'t it be great if you could show all your friends and family how much you love it? Well that\'s what we\'re here to offer today.\n\n# What We\'re Building\n\nWhy, T-Shirts of course! These beautiful shirts made out of 100% cotton and laser printed for long lasting goodness come from American Apparel. We\'ll be offering them in 4 styles:\n\n* Crew neck (wrinkled)\n* Crew neck (straight)\n* Scoop neck (fitted)\n* V neck (fitted)\n\nShirt sizings will be as follows:\n\n| Size   | S | M | L | XL |\n|--------|-----|-----|-----|------|\n| **Width**  | 18" | 20" | 22" | 24"  |\n| **Length** | 28" | 29" | 30" | 31"  |\n\n# Who We Are\n\nWe are the team behind grant.io. In addition to our software engineering experience, we have over 78 years of T-Shirt printing expertise combined. Sometimes I wake up at night and realize I was printing shirts in my dreams. Weird, man.\n\n# Expense Breakdown\n\n* $1,000 - A professional designer will hand-craft each letter on the shirt.\n* $500 - We\'ll get the shirt printed from 5 different factories and choose the best quality one.\n* $3,000 - The full run of prints, with 20 smalls, 20 mediums, and 20 larges.\n* $500 - Pizza. Lots of pizza.\n\n**Total**: $5,000',
     target: '5',
-    payoutAddress,
+    // Testnet address, assumes you wouldn't use this in production
+    payoutAddress:
+      'ztfFV7AqJqBm1EcWvP3oktZUMnp91ygfduE6ZQqGWENM1CpRKJLMZp2kgChnJVc6CbKSZ4mS37iNaiDwcatxjZcfoi2g7E8',
     milestones: [
       {
         title: 'Initial Funding',
@@ -36,7 +40,7 @@ const createExampleProposal = (payoutAddress: string): Partial<ProposalDraft> =>
         immediatePayout: false,
       },
     ],
-    deadlineDuration: 300
+    deadlineDuration: 300,
   };
 };
 

frontend/client/components/CreateFlow/index.tsx

@@ -103,7 +103,6 @@ interface StateProps {
   form: AppState['create']['form'];
   isSavingDraft: AppState['create']['isSavingDraft'];
   hasSavedDraft: AppState['create']['hasSavedDraft'];
-  accounts: string[];
 }
 
 interface DispatchProps {
@@ -161,7 +160,7 @@ class CreateFlow extends React.Component<Props, State> {
     let content;
     let showFooter = true;
     if (isSubmitting) {
-      content = <Final />;
+      content = <Final goBack={this.cancelSubmit} />;
       showFooter = false;
     } else if (isPreviewing) {
       content = <Preview />;
@@ -308,11 +307,12 @@ class CreateFlow extends React.Component<Props, State> {
     this.setState({ isShowingSubmitWarning: false });
   };
 
-  private fillInExample = () => {
-    const { accounts } = this.props;
-    const [payoutAddress] = accounts;
+  private cancelSubmit = () => {
+    this.setState({ isSubmitting: false });
+  };
 
-    this.updateForm(createExampleProposal(payoutAddress));
+  private fillInExample = () => {
+    this.updateForm(createExampleProposal());
     setTimeout(() => {
       this.setState({
         isExample: true,
@@ -324,12 +324,10 @@ class CreateFlow extends React.Component<Props, State> {
 
 const withConnect = connect<StateProps, DispatchProps, {}, AppState>(
   (state: AppState) => {
-    console.warn('TODO - remove/refactor accounts');
     return {
       form: state.create.form,
       isSavingDraft: state.create.isSavingDraft,
       hasSavedDraft: state.create.hasSavedDraft,
-      accounts: ['notanaccount'],
     };
   },
   {

frontend/client/modules/create/utils.ts

@@ -1,6 +1,6 @@
 import { ProposalDraft, CreateMilestone, STATUS } from 'types';
 import { User } from 'types';
-import { getAmountError } from 'utils/validators';
+import { getAmountError, isValidAddress } from 'utils/validators';
 import { MILESTONE_STATE, Proposal } from 'types';
 import { Zat, toZat } from 'utils/units';
 import { ONE_DAY } from 'utils/time';
@@ -81,8 +81,8 @@ export function getCreateErrors(
   }
 
   // Payout address
-  if (!payoutAddress) {
-    errors.payoutAddress = 'That doesn’t look like a valid address';
+  if (payoutAddress && !isValidAddress(payoutAddress)) {
+    errors.payoutAddress = 'That doesn’t look like a valid zcash address';
   }
 
   // Milestones

frontend/client/utils/validators.ts

@@ -17,7 +17,19 @@ export function isValidEmail(email: string): boolean {
   return /\S+@\S+\.\S+/.test(email);
 }
 
+// Uses simple regex to validate addresses, doesn't check checksum or network
 export function isValidAddress(address: string): boolean {
-  console.warn('TODO - implement utils.isValidAddress', address);
-  return true;
+  // T address
+  if (/^t[a-zA-Z0-9]{34}$/.test(address)) {
+    return true;
+  }
+  // Sprout address
+  if (/^z[a-zA-Z0-9]{94}$/.test(address)) {
+    return true;
+  }
+  // Sapling address
+  if (/^z(s)?(reg)?(testsapling)?[a-zA-Z0-9]{76}$/.test(address)) {
+    return true;
+  }
+  return false;
 }