Giter Site home page Giter Site logo

paystring's Introduction

PayString

License: Apache 2 Public API Version: 1.1 Admin API Version: 2020-08-28

Welcome to PayString, the universal payment identifier.

This project is not associated with PayID operated by NPP Australia Ltd. People in Australia are prohibited from using this project. See below for more details.

This is the reference implementation server for PayString, serving the PayString API. It uses TypeScript, a Node.js HTTP server, and a PostgreSQL database.

By default, the server hosts the Public API, which conforms to the PayString Protocol, on port 8080. The server also hosts a second RESTful API on port 8081 for CRUD (Create/Read/Update/Delete) operations to manage PayStrings and associated addresses.

To experiment with PayString, you can start a local server by running npm run devEnvUp, which uses our local docker-compose.yml file, which implicitly starts both a database and a PayString server inside Docker containers. To work on the PayString server source code itself, you can start a PostgreSQL database to develop against by running npm run devDbUp, which starts a database in a Docker container, and a local PayString server.

To clean up the associated Docker containers after you create a local server or database container, run npm run devDown.

Further Reading

Legal

By using, reproducing, or distributing this code, you agree to the terms and conditions for use (including the Limitation of Liability) in the Apache License 2.0. If you do not agree, you may not use, reproduce, or distribute the code. This code is not authorised for download in Australia. Any persons located in Australia are expressly prohibited from downloading, using, reproducing or distributing the code. This code is not owned by, or associated with, NPP Australia Limited, and has no sponsorship, affiliation or other connection with the “Pay ID” service operated by NPP Australia Limited in Australia.

paystring's People

Contributors

0xclarity avatar aanchal4 avatar dangell7 avatar dependabot-preview[bot] avatar dependabot[bot] avatar dino-rodriguez avatar florent-uzio avatar intelliot avatar keefertaylor avatar koresar avatar loisrp avatar mczochowski avatar mduo13 avatar mouradski avatar nhartner avatar nkramer44 avatar richardah avatar sappenin avatar stormtv avatar tedkalaw avatar tlongwell avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

paystring's Issues

Distinguish between `no payment pointer` and `no address for a payment pointer that exists`

Currently, we return a 404 and an identical error message for the cases of:

  • the payment pointer you specified does not exist
  • and, the payment pointer you specified does not have an address for the (payment_network, environment) tuple specified in the request header.

It's probably worth returning different error messages for these two cases to help consumers of PayID. Additionally, we may want to return a 406 - Not Acceptable in the second case where the payment pointer exists, but the address doesn't:

406: The resource identified by the request is only capable of generating response entities which have content characteristics not acceptable according to the accept headers sent in the request.

Ensure compliance with the latest White Paper

The white paper has a bunch of stuff about the public API, especially around error states, and the messages / error codes that should be passed back for different scenarios.

We should compare the implementation with the white paper to ensure compliance and either:

  • Bring the reference implementation into compliance with the white paper
  • Update the white paper to reflect the reference implementation.

A few places we may want to defer to the reference implementation over the white paper are if the reference implementation is using a better HTTP status code for a given error, or a better error message.

For example, the white paper has an error message for 400 - Bad Request for a missing certificate (for a signed public API handshake), with a message of "A certificate is required to verify the signature." But, it's often helpful to give the client an example of a good request, or a link to documentation to help them fix their problem. This is a case where we may want to update the white paper to reflect a better practice that we're using in the reference implementation.

This is related to #157

Metrics endpoint output format

Detailed Description

As of now, the format returned by the metrics endpoint (GET /metrics) seems to be tight to Prometheus:

# HELP payid_lookup_request count of requests to lookup a PayID
# TYPE payid_lookup_request counter
payid_lookup_request{paymentNetwork="XRPL",environment="TESTNET",org="payid.es",result="found"} 68
...
# HELP payid_count count of total PayIDs
# TYPE payid_count gauge
payid_count{paymentNetwork="BTC",environment="MAINNET",org="payid.app"} 1
payid_count{paymentNetwork="ETH",environment="TESTNET",org="payid.app"} 1
payid_count{paymentNetwork="XRPL",environment="MAINNET",org="payid.app"} 6
payid_count{paymentNetwork="XRPL",environment="TESTNET",org="payid.app"} 3

Request to abstract from the destination tool (Prometheus or whatever reporting tool is chosen) and adopt JSON as format so it is easier to read and transform.

Context

Easier processing by middlewares and reporting tools.

Make unit tests run just from npm run test

Today this fails with the error UnhandledPromiseRejectionWarning: Error: connect ECONNREFUSED 127.0.0.1:5432. Basically it is expecting us to already have a PG server in order for us to do anything. Would be nice if it just automatically started running a PG server upon that command.

Marking as bug because it just fails with unhandled promise error which would be a pretty rough UX for anybody new wanting to work on PayID.

Use `getUser` for the Public GET

The database code for getUser and getPaymentInfo is basically the same except for two where clauses that include a payment_network and a environment. Knex has optional query support, so this could be reduced to one query.

Once validation is in its own middleware as well, this would allow the Private/Public API to re-use a lot more of the same code.

Open to any thoughts @hbergren on what you might think about this.

Update docs to correct path to "demo" folder

The docs located at ( https://docs.payid.org/remote-deployment/ ) refer to a demo folder and a script within. The folder does not appear anywhere within the repo.

Detailed Description

Missing "demo" folder that docs refer to. At one point it seems it was renamed to "example" but that too has been removed in commit ( 34fef51 ).

Context

I am trying to step through the docs to set up a server on AWS and the folder is missing.

Keep separation of concerns between Data Access and Routing/Business Logic

We have a pretty good separation of concerns between the two, but we have things like this:

  try {
    await insertUser(paymentPointer, req.body.addresses)
  } catch (err) {
    // TODO(hbergren): This leaks database stuff into this file
    // This probably means error handling should be done in the data access layer
    if (
      err.message.includes(
        'violates unique constraint "account_payment_pointer_key"',
      )
    ) {
      return handleHttpError(
        409,
        `There already exists a user with the payment pointer ${paymentPointer}`,
        res,
        err,
      )
    }

This clearly breaks the separation of concerns, since we're matching on an error raised by the database. This should stay in the data access layer, and we should figure out a way to communicate the appropriate status code and error message to return.

This might look like coming up with a custom enum/list of custom DatabaseError errors, the same way that the xpring-sdk defines custom errors for IlpError and XrpError. Then in the routing/business-logic layer, we could pattern match on the relevant error.

Logs for all requests (not just errors) on debug

Right now, logs only appear on errors. It would be nice if it were possible to set logs to show on all requests, as it would make it easier to debug errors that happen before or after payid is hit by requests.

I'm trying to wire up PayID, and requests on the private API succeed but fail on the public API. In my case, the issue is a problem with the configuration of my server in GKE. If I could see all incoming requests, I would be able to more quickly determine that something else is going on

Reason about case-sensitivity of payment pointers throughout application

I think we're case-sensitive to payment pointers right now, but that is technically incorrect.

Since payment pointers are just another representation of a HTTPS URL, we need to follow the case-sensitivity of URLs. Which is treating the path of the URL as case sensitive, but not the domain.

So, for example, we should write some logic/tests to assert that:

  • $XPRING.MONEY/hbergren and $xpring.money/hbergren both evaluate correctly to the same payment pointer / address
  • $xpring.money/HBERGREN and $xpring.money/hbergren do NOT evaluate to the same payment pointer / address.

Automatic Deployment of Development Database

Currently we expect developers to set up their own local postgres instance. This is a sharp edge because:

  1. it's manual work
  2. we aren't clear that this is an expectation in our docs

I think there are two paths for this:

  1. npm i --save-dev sqlite3 and just look for an ENV variable indicating staging or prod, and if it doesn't exist fall back to a local sqlite DB (easiest path)
  2. Deploy a local postgres DB within a docker container.
    "Part of the thinking here is that you never switch database systems, so we can write SQL that is optimized for Postgres. We probably can't just spin up a sqlite database for this." - @hbergren (more optimized path)

201 Responses should include a `Location` header

For the private API, POST and PUT can return a 201 - Created response. When they do, we should return a Location header that tells the client where they should direct GET requests to retrieve the created resource.

Reduce `try/catch` Statements (Better Error Handling)

Currently we have try/catch statements all throughout the code returning lots of different kinds of errors. Ideally this error handling is refactored to make the code more readable.

A few possible approaches:

  • Error handling helpers that contain try/catch
  • Catching errors at the lowest levels (in the database layer services for example), and returning Either Monads in those functions

Implement `PATCH` for `/users/{payid}`

Right now, it’s a huge hassle to update someone’s PayID. You have to GET xpring.money:8081/hbergren$xpring.money

Which gives you a payload like this:

{
  "payId": "hbergren$xpring.money",
  "addresses": [...],
}

And then you have to update the payId field in that response payload, and send it back to the server using:
PUT xpring.money:8081/hbergren$xpring.money (including the payload).

Proposed Solution

What would be great is to have something like this:

PATCH xpring.money:8081/hbergren$xpring.money

{
  "payId": "hansbergren$xpring.money"
}

And have that just update the PayID of the user.

Requirements

  • Updating the payId of a user should result in a 201 - Created response
  • Because the resource lives at a new place, we should include a Location header
  • E2E tests for the PATCH method should be implemented
  • We should require a request header Content-Type: application/merge-patch+json
  • The Admin API server should set a Accept-Patch: application/merge-patch+json header.

Implement a `GET /users` endpoint for the private API

Right now, you can fetch all data associated with a specific payment pointer, but you can't retrieve/search/query across all users.

If we did this, we would need to think about the querystring parameters to use for filtering/search, as well as pagination of results.

Implement `@types` package for the public interface of PayID

@dino-rodriguez mentioned we could ship a @types package in DefinitelyTyped so that other TS applications that hit a PayID service can import the types our public & private interface accept and return.

That seems worth doing.

Note that this may not actually be in DefinitelyTyped, it might just be a payid-schema package that we publish on NPM.

Request/Request Body Validation (Private API)

Requests and request bodies should be validated for the Private API. This is lower priority than any work on the public API because it is not client facing.

  • Private API
    • Headers
    • URL
    • Body

ENV var for running seed script

currently npm run start does not run the seed script while npm run test does. This is an issue because the Xpring-JS SDK repo depends on dev.payid.xpring.money having the seeded values. We should add an env var (default to development) that seeds the database when running index.js

Always send a JSON response?

Not sure if we need to do this or not, but currently, when we send a 201 - Created or a 200 - OK without a response body, we return a Content-Type: text/plain response. That might be ok, but it might be reasonable to always return JSON, since we're basically exposing a JSON RESTful API.

More Unit Tests

Not all functions are covered. Should include more unit tests where possible to iron out smaller bugs.

Dedicated Logging

Use a dedicated logger with different log levels instead of the console. Considering winston.

Win 10 Pro Docker install - payid missing file error 127

Expected Behavior

Install Docker for Windows
From PowerShell adm run devEnvUp
Install and run both payid and payid-database
Both machines run

Actual Behavior

At step 4, payid does not start and yields the following erro:

/usr/local/bin/docker-entrypoint.sh: exec: line 8: /opt/payid/scripts/wait-for-postgres.sh: not found

Environment

  • Node version: 12.18.1
  • NPM version: 6.14.5
  • Operating System and version: Windows 10 Pro v 2004
  • PayID server version: 1.0.0
  • PayID Version header (if applicable):

Figure out what to do about accounts with no associated addresses

This might just fall under #165, of writing more E2E tests, but right now, there are no seeded values / tests around a user account that has no associated addresses.

Maybe we should just disallow this case by using validation when you POST or PUT a user, or this is a valid thing that can happen, and we should write some E2E and unit tests to make sure we handle it appropriately.

Maybe: Expose nested subresources under user?

It may make sense for the private API to expose nested resources under user.

For example, GET /v1/users/$xpring.money/hbergren/addresses/xrpl-testnet

Then you could imagine the following "helper methods":

  • POST /v1/users/$xpring.money/hbergren/addresses/
  • PUT /v1/users/$xpring.money/hbergren/addresses/xrpl-testnet
  • PATCH /v1/users/$xpring.money/hbergren/addresses/xrpl-testnet
  • DELETE /v1/users/$xpring.money/hbergren/addresses/xrpl-testnet

This is probably related to #142 (implementing PATCH for the private API), and maybe even #141.

Remove hardcoded/default values

There are quite a few hardcoded/default values throughout the codebase that we should remove to make this "real".

Some concrete examples:

  • In the public API paymentNetwork is hardcoded to XRPL.
  • https:// is hardcoded in the public GET API, but it should not be
    • This one may need to be ENV var driven, because I'm not sure if we can do E2E tests over HTTPS normally.
  • We currently have a hardcoded check on the accept header for xrpl, but we probably shouldn't. Either we can keep a whitelist of Accept headers and return a 415 - Bad Mime Type if we get an unknown one. Or (and probably better), we can be optimistic and just always parse the Accept header and query the database anyways. Regardless, the current implementation is incorrect.
  • We hardcode environment to TESTNET for no reason.
  • We hardcode organization = 'xpring' in all the private API data access functions. This might be ok, but ideally, this is driven by matching some API token (Bearer-Auth?) to an organization table in the DB?

Make public GET API optimistic about `Accept` headers

Currently, we whitelist xrpl Accept headers, but there's no reason to. We can be optimistic and always parse the Accept header, and always query the database.

If we really want to maintain a whitelist of valid MIME types for PayID (not recommended), then we should at least return a 415 - Bad Mime Type.

Can't run devDbUp, missing script

Expected Behavior

Install Docker and clone repository
Install gitBash, Node.js
run devDbUp
Start working

Actual Behavior

Installed Docker, Node.js, and Git Bash.
Try npm run devDbUp
Says missing "script"

Context

This issue has pervented me from starting to worrk and develop.
I had as my first issue not having my SSH keys connected to github I fixed that then I had to create a package.json file in my users. After I did that my new issue was that I could not run "npm run devDbUp" or "npm run devEnvUp".

Steps to Reproduce

  1. Install Git Bash and Node.js
  2. run: npm install
  3. run: npm install https://github.com/payid-org/payid
  4. open powershell and run: npm run devEnvUp

Environment

  • Node version: 12.18.1
  • NPM version: 6.14.5
  • Operating System and version: Windows 10 Pro, 19041.329
  • PayID server version: n/a
  • PayID Version header (if applicable): n/a

Screenshots

image

Use TypeScript to its fullest extent (Type things better)

There are various places where things are typed simply as string, whereas we know there are some constraints on what they actually are.

For example, an account_id is a uuid, not just any string. A payment_pointer is always a $payment_pointer, whereas a payment_pointer_url is always an https:// style URL.

This is pretty similar to #129 and #130, but instead of just validating, this would mean getting better type-safety throughout the codebase, instead of just validating incoming payloads.

JSDoc Comments

All functions should have JSDoc comments on them. Important for understanding code at a glance.

Add more E2E tests

When we get better about validation #127, #129, we should implement more E2E tests for the public and private API to assert correct error codes / error messages are being thrown.

I know for a fact we have no 400 E2E test for the private GET API, but I'm sure we're missing various cases for both the public and private API.

Request Validation (Public API)

Request contents should be validated to make sure they match an appropriate schema. More specifically:

  • Public API
    • Headers
    • URL

Standardize error messages across the app

Similar to JSDoc comments #131 and code comments #132, we should do a pass through all the error messages we return in the app and just make sure we're being consistent in our communication, and following best practices for error messages.

Best practices for error messages in an API include a link to documentation for Bad Requests, and possibly an example good request so the client knows how to fix the problem.

Maybe: Take a dependency on `xpring-common-js`/`xpring-js`.

We define some helper utility methods for payment pointer conversion, and some TypeScript interfaces that define appropriate request/response payloads.

It may be worth moving those down into xpring-common-js, since it also needs the conversion functions, and that way we aren't duplicating code / can have one solidly battle-tested implementation of this stuff.

Related: #144, since if we do that, maybe xpring-common-js can just expose the canonical PayID TS interfaces, and we may not need a separate types/payid-interface package (although we may want still one).

PayID format needs to be standardised, regex currently wrong

https://github.com/xpring-eng/payid/blob/f85b0fd3386a6fabb1aeae6e65fe89ae31db9078/src/db/schema/01_account.sql#L20

Hi guys! This regular expression is clearly wrong for example it matches

payid$example.com/garbage?more=garbage&even=more#garbage

To avoid reinventing the wheel I suggest that PayID is already very similar to the email address standard defined in RFC2822. All that needs to be done to convert this existing standard to a PayID standard is to swap $ for @ and @ for $. That's it. As an added bonus this would allow most email addresses to be used as the first half of a PayID (i.e. as an identifier.)

The downside to RFC2822 addresses is that attempting to mirror the exact standard with a regular expression results in extremely long regular expressions. However you can match 99.99% of valid email addresses with relatively short expressions. The internet is full of these and on-going debates about which one is the best. I propose to use the standardised one that is included in RegexBuddy 4 with some modifications.

The original RegexBuddy 4 regex for RFC2822 (simplified) looks like this:

[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?

First remove all quote characters as these invite only problems and swap $ with @

^[a-z0-9!#@%&*+/=?^_`{|}~-]+(?:\.[a-z0-9!#@%&*+/=?^_`{|}~-]+)*\$(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z-]*[a-z0-9])?$

Next restrict TLDs so they don't allow numeric TLDs and then provide a second branch that allows specification of an IPv4 IP instead of a punycode domain. (We might also want to support IPv6 or remove this branch to support no IPs as hostnames):

^[a-z0-9!#@%&*+/=?^_`{|}~-]+(?:\.[a-z0-9!#@%&*+/=?^_`{|}~-]+)*\$(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z-]*[a-z0-9])?|(?:[0-9]{1,3}\.){3}[0-9]{1,3})$

Now we're done, time to test. I see you guys use postgre so I tested there.

-- old regex is: '^([a-z0-9\-\_\.]+)(?:\$)[^\s/$.?#].+\.[^\s]+$'
-- new regex is: '^[a-z0-9!#@%&*+/=?^_`{|}~-]+(?:\.[a-z0-9!#@%&*+/=?^_`{|}~-]+)*\$(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z-]*[a-z0-9])?|(?:[0-9]{1,3}\.){3}[0-9]{1,3})$'

drop table if exists payid_examples;
create table payid_examples ( pay_id varchar(250) primary key, is_valid bool );
-- valid payids
insert into payid_examples values ( '1$1.1.1.1', true);
insert into payid_examples values ('payid$example.com', true);
insert into payid_examples values ('firstname.lastname$example.com', true);
insert into payid_examples values ('payid$subexample.example.com', true);
insert into payid_examples values ('firstname+lastname$example.com', true);
insert into payid_examples values ('payid$123.123.123.123', true);
insert into payid_examples values ('1234567890$example.com', true);
insert into payid_examples values ('payid$example-one.com', true);
insert into payid_examples values ('_______$example.com', true);
insert into payid_examples values ('payid$example.name', true);
insert into payid_examples values ('payid$example.co.jp', true);
insert into payid_examples values ('firstname-lastname$example.com', true);
insert into payid_examples values ('[email protected]$example.com', true);
insert into payid_examples values ('[email protected]$example.com', true);
insert into payid_examples values ('[email protected]$example.com', true);
insert into payid_examples values ('[email protected]$example.com', true);
insert into payid_examples values ('[email protected]$example.com', true);
insert into payid_examples values ('[email protected]$example.com', true);
insert into payid_examples values ('[email protected]$example.com', true);
insert into payid_examples values ('[email protected]$example.com', true);
insert into payid_examples values ('[email protected]$example.com', true);
insert into payid_examples values ('[email protected]$example.com', true);
insert into payid_examples values ('[email protected]$example.com', true);
insert into payid_examples values ('[email protected]$example.com', true);
-- invalid payids
insert into payid_examples values ('payid@[123.123.123.123]$example.com', false);
insert into payid_examples values ('payid$[123.123.123.123]', false);
insert into payid_examples values ('"payid"$example.com', false);
insert into payid_examples values ('#$%^%#$$#$$#.com', false);
insert into payid_examples values ('$example.com', false);
insert into payid_examples values ('Joe Smith <payid$example.com>', false);
insert into payid_examples values ('payid.example.com', false);
insert into payid_examples values ('payid$example$example.com', false);
insert into payid_examples values ('.payid$example.com', false);
insert into payid_examples values ('payid.$example.com', false);
insert into payid_examples values ('payid..payid$example.com', false);
insert into payid_examples values ('あいうえお$example.com', false);
insert into payid_examples values ('payid$example.com (Joe Smith)', false);
insert into payid_examples values ('payid$example', false);
insert into payid_examples values ('payid$-example.com', false);
insert into payid_examples values ('payid$111.222.333.44444', false);
insert into payid_examples values ('payid$example..com', false);
-- really invalid payids
insert into payid_examples values ('payid$example.com?garbage', false);
insert into payid_examples values ('payid$example.com/garbage', false);
insert into payid_examples values ('payid$example.com#garbage', false);
insert into payid_examples values ('payid$example.com&garbage', false);
insert into payid_examples values ('payid$example.com:garbage', false);
insert into payid_examples values ('payid$example.com\\garbage', false);
insert into payid_examples values ('payid$example.com/garbage?more=garbage&even=more#garbage', false);
insert into payid_examples values ('payid$example.com;', false);
insert into payid_examples values (E'payid$example.com\'', false);
insert into payid_examples values ('payid$example.com"', false);
insert into payid_examples values ('p$ayid$example.com?garbage', false);

-- test old regex

select count(*) from payid_examples where (pay_id ~ '^([a-z0-9\-\_\.]+)(?:\$)[^\s/$.?#].+\.[^\s]+$') != is_valid;

-- test new regex

select count(*) from payid_examples where (pay_id ~ '^[a-z0-9!#@%&*+/=?^_`{|}~-]+(?:\.[a-z0-9!#@%&*+/=?^_`{|}~-]+)*\$(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z-]*[a-z0-9])?|(?:[0-9]{1,3}\.){3}[0-9]{1,3})$') != is_valid;


-- create table using this regex as constraint

DROP TABLE IF EXISTS account;
CREATE TABLE account (
	pay_id varchar(200) primary key,

	-- AUDIT COLUMNS
	created_at timestamp with time zone NOT NULL DEFAULT(CURRENT_TIMESTAMP),
	updated_at timestamp with time zone NOT NULL DEFAULT(CURRENT_TIMESTAMP),

	-- CONSTRAINTS
	CONSTRAINT pay_id_length_nonzero CHECK(length(pay_id) > 0),
	CONSTRAINT pay_id_lowercase CHECK(lower(pay_id) = pay_id),

        CONSTRAINT valid_pay_id CHECK(pay_id ~ '^[a-z0-9!#@%&*+/=?^_`{|}~-]+(?:\.[a-z0-9!#@%&*+/=?^_`{|}~-]+)*\$(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z-]*[a-z0-9])?|(?:[0-9]{1,3}\.){3}[0-9]{1,3})$')
);

-- test table constraint by inserting valid examples
insert into account select pay_id from payid_examples where is_valid = true;

select * from account;

I'll create a PR with my proposed constraint change shortly and link it here.

Identify `fatal` errors and log and kill the program appropriately

In some cases (like some private API data access methods), we kill the program and log when a DB unique constraint would have been violated (for example, two users with the same payment pointer).

However, there are more unique constraints in the DB, and we don't log and call process.exit(1) in all places where we probably should.

A concrete example would be the public API data access method, where there should always only ever be zero or one rows returned. Multiple rows returned indicates we are in uncharted waters, and we should halt the program.

PUTs that update the `payment_pointer` should return a `200` with a `Location` header

Since we use the payment_pointer to identify a user resource, but PUT can update a payment_pointer, when we update the resource using PUT, we should return a 200, and include a Location header that links to the URL with the new payment_pointer.

EDIT

I was wrong earlier about using any type of 3XX status code:

3xx Redirection
This class of status code indicates that further action needs to be taken by the user agent in order to fulfill the request. The action required MAY be carried out by the user agent without interaction with the user if and only if the method used in the second request is GET or HEAD. A client SHOULD detect infinite redirection loops, since such loops generate network traffic for each redirection.

No further action needs to be taken by the user agent to fulfill a PUT request that updates a payment pointer, but we SHOULD return a Location header that links to the new resource.

Misc

This totally breaks the RESTful API principle that PUT should be an idempotent operation... oh well.

PayID Server-Side Forwarding

Hi i want to suggest payID forwarding.

That means that I want to be able to store my payid of e.g. bitrue (username$bitrue.com) behind my
very own payID. So if some resolver (like XUMM) sets out a requests for gafo666_bitrue$pid.gafo666.me
it should return the address written in e.g. username$bitrue.com .

Greetings GAFO666

Payment Pointer Validation Middleware/Helpers

Repeated code when validating Payment Pointers received in the request. This should happen in a middleware anytime a request is received, as opposed to being repeated throughout individual functions.

DB Migrations

Currently we do not have migrations functionality. This is now becoming an issue because we are updating the account schema to reflect the paymentPointer -> payId refactor.

We need to decide whether to implement this today or punt on it. If we punt it looks like we'll have to nuke the stage/prod PayID DB's running in Xpring currently and migrate all of the data from the wallet's alias system. I think it's worth considering if this data transfer might take even more time than just writing the migration functionality.

From @hbergren:
Add a db/migrations directory
Add migrations to the start of the array of directories that we scan through when we execute SQL in syncDatabaseSchema
Write something like this:

-- filename: 01_payment_pointer_to_pay_id_migration.sql

ALTER TABLE account IF EXISTS
RENAME payment_pointer TO pay_id;

-- Probably drop and recreate the renamed constraints here,
-- unless you can actually rename a constraint.
-- And wrap this whole thing in a transaction.

Then the migration would run automatically. You also wouldn't need to SSH into stage/prod. After the migration had run, we could even delete the migration file.

Code Comments

Parts of the code that require a human readable touch should include code comments. This is a pretty broad issue. One initial pass should cover it, and then we can move forward doing this (same for JSDocs).

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.