Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISO-8583 PoC Milestone 1 #1001

Merged
merged 3 commits into from
Sep 20, 2023
Merged

ISO-8583 PoC Milestone 1 #1001

merged 3 commits into from
Sep 20, 2023

Conversation

dastansam
Copy link
Contributor

@dastansam dastansam commented Sep 15, 2023

Milestone Delivery Checklist

  • The milestone-delivery-template.md has been copied and updated.
  • The invoice form 📝 has been filled out for this milestone.
  • This pull request is being made by the same account as the accepted application.
  • I have disclosed any and all sources of reused code in the submitted repositories and have done my due diligence to meet its license requirements.
  • In case of acceptance, the payment will be transferred to the BTC/ETH/fiat account provided in the application.
  • The delivery is according to the Guidelines for Milestone Deliverables.

Link to the application pull request: w3f/Grants-Program#1809

@dastansam dastansam marked this pull request as ready for review September 15, 2023 03:33
@stojanov-igor
Copy link
Contributor

HI @dastansam,

I provided an external evaluation for your delivery here: #1003

There were a few issues to be resolved before accepting the delivery.

@dastansam
Copy link
Contributor Author

dastansam commented Sep 18, 2023

HI @dastansam,

I provided an external evaluation for your delivery here: #1003

There were a few issues to be resolved before accepting the delivery.

hey @stojanov-igor,

thanks for the evaluation.

I have couple of questions before I address your review.

  1. What is the OS that you are running?
  2. Do you have postgres installed and running? What are the arguments you provided to the oracle? Specifically for DB args?

By default, it tries to connect to postgres DB at (it's documented at README):

      --database-host <DATABASE_HOST>
          Path to the Postgres database [default: localhost]
      --database-port <DATABASE_PORT>
          Port of the Postgres database [default: 5432]
      --database-user <DATABASE_USER>
          Username of the Postgres database [default: postgres]
      --database-name <DATABASE_NAME>
          Name of the Postgres database [default: postgres]

And if you have different postgres config, you should pass them explicitly.

@stojanov-igor
Copy link
Contributor

What is the OS that you are running?

Running on MacOS Monterey Version 12.5.1

Do you have postgres installed and running? What are the arguments you provided to the oracle? Specifically for DB args?

Do not have Postgres installed. Can you add this to the Readme as a prerequisite?

Also I am providing no arguments to the oracle.

@dastansam
Copy link
Contributor Author

What is the OS that you are running?

Running on MacOS Monterey Version 12.5.1

Do you have postgres installed and running? What are the arguments you provided to the oracle? Specifically for DB args?

Do not have Postgres installed. Can you add this to the Readme as a prerequisite?

Also I am providing no arguments to the oracle.

Postgres is already mentioned as a pre-requisite

@nikw3f
Copy link
Contributor

nikw3f commented Sep 19, 2023

I would recommend to create a docker-compose file and create a simple system diagram to show how everything is working together.

@dastansam
Copy link
Contributor Author

dastansam commented Sep 19, 2023

I would recommend to create a docker-compose file and create a simple system diagram to show how everything is working together.

hey @nikw3f,

I added full docker-compose support (although it wasn't part of the deliverable) and also updated docs with the high-level diagram of the infrastructure as you suggested. This is the PR that adds the changes: subclone/payment-processor#2

Looking forward to your evaluation.

@nikw3f
Copy link
Contributor

nikw3f commented Sep 20, 2023

Hey team, one of the test is failing. Can you please check:

cargo test
warning: profiles for the non root package will be ignored, specify profiles at the workspace root:
package:   /Users/nikhil/w3f/payment-processor/pcidss/oracle/Cargo.toml
workspace: /Users/nikhil/w3f/payment-processor/pcidss/Cargo.toml
    Finished test [unoptimized + debuginfo] target(s) in 0.12s
     Running unittests src/lib.rs (target/debug/deps/op_api-4948e635d4fc3069)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/op_core-4ed3cc35f909a678)

running 5 tests
test transaction::models::tests::test_conversion_from_transaction_create_to_transaction ... ok
test bank_account::models::tests::test_successful_credit ... ok
test bank_account::models::tests::test_successful_debit ... ok
test bank_account::models::tests::test_arithmetic_overflow_nonce ... ok
test bank_account::models::tests::test_arithmetic_overflow_balance ... ok

test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/main.rs (target/debug/deps/pcidss_oracle-56e2f38d67c88873)

running 2 tests
test tests::payment::test_payment ... FAILED
test tests::reversal::test_reversals_success ... ok

failures:

---- tests::payment::test_payment stdout ----
thread 'tests::payment::test_payment' panicked at 'Error to init database to tests: InternalServerError("db error: ERROR: duplicate key value violates unique constraint \"pg_database_datname_index\"\nDETAIL: Key (datname)=(mockdb) already exists.")', oracle/src/tests/mock.rs:25:41
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    tests::payment::test_payment

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.42s

error: test failed, to rerun pass `-p pcidss-oracle --bin pcidss-oracle`

@dastansam
Copy link
Contributor Author

Hey team, one of the test is failing. Can you please check:

cargo test
warning: profiles for the non root package will be ignored, specify profiles at the workspace root:
package:   /Users/nikhil/w3f/payment-processor/pcidss/oracle/Cargo.toml
workspace: /Users/nikhil/w3f/payment-processor/pcidss/Cargo.toml
    Finished test [unoptimized + debuginfo] target(s) in 0.12s
     Running unittests src/lib.rs (target/debug/deps/op_api-4948e635d4fc3069)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/op_core-4ed3cc35f909a678)

running 5 tests
test transaction::models::tests::test_conversion_from_transaction_create_to_transaction ... ok
test bank_account::models::tests::test_successful_credit ... ok
test bank_account::models::tests::test_successful_debit ... ok
test bank_account::models::tests::test_arithmetic_overflow_nonce ... ok
test bank_account::models::tests::test_arithmetic_overflow_balance ... ok

test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/main.rs (target/debug/deps/pcidss_oracle-56e2f38d67c88873)

running 2 tests
test tests::payment::test_payment ... FAILED
test tests::reversal::test_reversals_success ... ok

failures:

---- tests::payment::test_payment stdout ----
thread 'tests::payment::test_payment' panicked at 'Error to init database to tests: InternalServerError("db error: ERROR: duplicate key value violates unique constraint \"pg_database_datname_index\"\nDETAIL: Key (datname)=(mockdb) already exists.")', oracle/src/tests/mock.rs:25:41
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    tests::payment::test_payment

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.42s

error: test failed, to rerun pass `-p pcidss-oracle --bin pcidss-oracle`

Hey @nikw3f

I just pushed a change that fixes the failing test.

It was because of two tests trying to connect to the same DB, you could run with cargo test --test-threads=1 to get away with it, but I pushed a fix that makes it work with cargo test

Let me know if you still see this issue.

@nikw3f
Copy link
Contributor

nikw3f commented Sep 20, 2023

Thanks for the delivery @dastansam Good Work! You can find my finished evaluation here. Happy to pass the milestone, and looking forward to the substrate implementation.

@nikw3f nikw3f merged commit 078d0ee into w3f:master Sep 20, 2023
6 checks passed
@github-actions
Copy link

Congratulations on completing the first milestone of this grant! As part of the Grants Program, we want to help grant recipients acknowledge their grants publicly. To that end, we’ve created a badge for projects that successfully deliver their first milestone. Please use the badge only in reference to the work that has been completed as part of this grant, so please do not display it on your team or project's homepage unless accompanied by a short description of the grant. Furthermore, you're now welcome to announce the grant publicly. Please remember to observe the foundation’s guidelines in doing so. If you haven't already, reach out to [email protected] for feedback on your announcement and cross-promotion.

Thank you for your contribution, and good luck! If you have any remaining milestone, let us know if you encounter any delays by leaving a comment on the application PR or submitting an amendment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants