[DEVOPS-834] Cardano faucet package #2939

commandodev · 2018-05-16T14:19:59Z

Description

Faucet for the cardano testnet.

Linked issue

DEVOPS-834

Type of change

🐞 Bug fix (non-breaking change which fixes an issue)
🛠 New feature (non-breaking change which adds functionality)
⚠️ Breaking change (fix or feature that would cause existing functionality to change)
🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc
🔨 New or improved tests for existing code
⛑ git-flow chore (backport, hotfix, etc)

Developer checklist

I have read the style guide document, and my code follows the code style of this project.
If my code deals with exceptions, it follows the guidelines.
I have updated any documentation accordingly, if needed. Documentation changes can be reflected in opening a PR on cardanodocs.com, amending the inline Haddock comments, any relevant README file or one of the document listed in the docs directory.

Testing checklist

I have added tests to cover my changes.
All new and existing tests passed.

cleverca22 · 2018-05-22T15:23:34Z

default.nix

@@ -64,7 +64,8 @@ let
        executableHaskellDepends = drv.executableHaskellDepends ++ [self.cabal-install];
      })));
      cardano-sl-node = addGitRev super.cardano-sl-node;
-      cardano-sl-wallet-new = addGitRev (justStaticExecutables super.cardano-sl-wallet-new);
+      cardano-sl-wallet-new = addGitRev super.cardano-sl-wallet-new;
+      cardano-sl-wallet-new-static = justStaticExecutables cardano-sl-wallet-new;


upssing a super. on this line

cleverca22 · 2018-05-22T15:26:18Z

default.nix

-        # Static linking for everything to work around
-        # https://ghc.haskell.org/trac/ghc/ticket/14444
-        # This will be the default in nixpkgs since
-        # https://github.com/NixOS/nixpkgs/issues/29011


leave the comments in, and move enableSharedExecutables out of the args ? src block

cleverca22 · 2018-05-22T15:27:59Z

faucet/default.nix

+, servant-swagger-ui, stdenv, swagger2, text, text-format, tls, wai
+, wai-cors, wai-extra, warp
+}:
+mkDerivation {


can probably delete this file, stack2nix handles it automatically

This was for convenience while I was developing (to get round having to regenerate pkgs/default.nix whenever I changed dependencies. I'll keep it for now, but will make a note to get rid of it and shell.nix before merging.

cleverca22

currently only looking at the nix files

I think I've addressed everything apart from this informational "Only looking at the nix files" message?

erikd · 2018-07-14T02:48:23Z

faucet/generated-wallet-details.json

@@ -0,0 +1 @@
+{"wallet-id":"Ae2tdPwUPEZJkLTchA9mETtyXumBT1K1gqtX3dgXUguJFLuvyNU9BzQXrEK","recovery-words":["relax","secret","seek","story","supreme","meat","judge","onion","weasel","sponsor","garlic","erase"],"address":"DdzFFzCqrhswmBoqDubnsZZpvnfFbjgwLwFyk4vC5jJJf75m36f4M3f67k4iia1RQdCUPmJHV7c9TK2ogfzjoQSYHKCoM4YTv6zrmr28","account-index":2147483648}


Is it worth pretty printing that JSON so its a bit more human readable?

That wasn't actually supposed to be in there. I've added it to ignore.

erikd · 2018-07-14T02:49:26Z

faucet/server/Main.hs

+      _statsd <- liftIO $ forkStatsd (config ^. fcStatsdOpts . _Wrapped') (fEnv ^. feStore)
+      liftIO $ run (config ^. fcPort) (serve faucetDocAPI server)
+  where
+    -- runLogger :: LoggerNameBox IO a -> IO a


Comment? Why not actually have the type sig there so the compiler can validate that it is indeed correct.

rvl

Split up the withdraw function.
Can we have at least one token test case?
Needs a little documentation.
Remove _dead code and commented out code
Remove nix files and .projectile.
Remove index.html
Copyright IOHK
There is a missing letter in Withdrawl

commandodev · 2018-07-20T08:37:56Z

@rvl these would be easier to address if they were inline :-)

Split up the withdraw function.

I've split out the queue handling parts 59a71c7

Can we have at least one token test case?

I'm sure there are some parts that can be sensibly tested now

Needs a little documentation.

There are haddocks on all the main functions and swagger docs (under /docs). What form would you like the documentation in? Do you mean a better README?

Remove _dead code and commented out code

I've removed all the commented code I could find, could you review inline anymore you see and anything you think is dead? 7cabdb9

Remove nix files and .projectile.

Done a015867

Remove index.html

This is kinda handy to have around for testing purposes... Now that recaptcha is enabled you can't really test with curl or a rest client

Copyright IOHK

Done - found the remaining instance in LICENSE 3d945c6

There is a missing letter in Withdrawl

Urban dictionary informs me that "withdrawl" is

What a Southerner suffers when he moves to the North

which isn't quite what I intended. Fixed in 1c2ae37

commandodev · 2018-07-20T15:43:32Z

Made a start on a test suite in 8741c9d

rvl · 2018-07-23T13:36:04Z

faucet/cardano-sl-faucet.cabal

+                    , Cardano.WalletClient
+  default-language: Haskell2010
+
+executable faucet


It would help with deployment if this were called cardano-faucet.

Sure - I can change that easy enough.

rvl · 2018-07-23T13:52:15Z

faucet/server/Main.hs

+
+main :: IO ()
+main = withCompileInfo $ do
+    ekg <- forkServer "localhost" 8001


This needs to be configurable with --ekg-server option, like the wallet.
You might be able to re-use option handling code from Pos.Infra.Statistics.Ekg.

rvl · 2018-07-24T00:23:12Z

In the config file, can the recaptcha secret key be specified as a path to a file from which the key will be read, rather than stored directly in the JSON?

rvl · 2018-07-25T00:03:13Z

Thanks.

~~Can readCaptchaSecret read just the first line of the file and not include the newline characters?~~ (edit: never mind -- it seems to work -- the newline character is removed somewhere).

Helps when testing locally using curl. If there is a recaptcha secret configured, it will fail the recaptcha check and return HTTP 400.

rvl · 2018-08-07T11:27:47Z

Ported faucet branch from develop to release/1.3.0.

Previous version of faucet which is based off develop is on the devops-834-faucet-develop branch.

commandodev requested review from adinapoli-iohk, akegalj, erikd and iohk-devops as code owners May 16, 2018 14:19

commandodev force-pushed the faucet branch from f28315d to 75b1c63 Compare May 22, 2018 10:37

cleverca22 reviewed May 22, 2018

View reviewed changes

cleverca22 previously requested changes May 22, 2018

View reviewed changes

rvl force-pushed the faucet branch from 93099fd to 954d8bb Compare July 2, 2018 04:59

commandodev force-pushed the faucet branch 4 times, most recently from 686994b to 06ad698 Compare July 13, 2018 16:41

erikd reviewed Jul 14, 2018

View reviewed changes

commandodev force-pushed the faucet branch 2 times, most recently from 88fde4e to 2ce31cf Compare July 16, 2018 14:39

rvl reviewed Jul 19, 2018

View reviewed changes

commandodev force-pushed the faucet branch 3 times, most recently from 49b5656 to d323db0 Compare July 23, 2018 09:59

rvl reviewed Jul 23, 2018

View reviewed changes

commandodev force-pushed the faucet branch from cdc5501 to efd2112 Compare July 24, 2018 06:49

rvl and others added 19 commits August 7, 2018 10:23

[DEVOPS-834] Add demo frontend to faucet

bdb862b

[DEVOPS-834] Fix faucet static serving

af590fe

[DEVOPS-834] Apply stylish haskell fixes

a7dd151

[DEVOPS-834] Add universium and fix indentation/sorting of deps

5cb8f07

[DEVOPS-834] Switch to universum

dd6899f

[DEVOPS-834] Switch to safe-exceptions

bf1f14a

[DEVOPS-834] Fix for upstream API changes

706e8bb

[DEVOPS-834] Apply stack2nix patch

73de465

[DEVOPS-834] Apply stylish.diff from hydra

cae4607

[DEVOPS-834] Remove existing wallet json file

78eba05

[DEVOP-834] Add -Wall everywhere

c79560b

[DEVOPS-834] Use a pattern match instead of whenJust

b0eaa78

[DEVOPS-834] Remove unused decrWithdrawn function

0eb2aa5

[DEVOPS-834] Remove unused DepositRequest and DepositResult

2bb288b

[DEVOPS-834] Remove -Wall from the modules

c941600

[DEVOPS-834] Fix some warnings

e6c8338

[DEVOPS-834] faucet: Backport to release/1.3.0

71b43fe

[DEVOPS-834] Regenerate pkgs/default.nix

5a57bb4

[DEVOPS-834] faucet: make recaptcha optional in WithdrawalRequest

ce421d8

Helps when testing locally using curl. If there is a recaptcha secret configured, it will fail the recaptcha check and return HTTP 400.

rvl force-pushed the faucet branch from 939590c to ce421d8 Compare August 7, 2018 11:24

rvl changed the base branch from develop to release/1.3.0 August 7, 2018 11:24

rvl added DO NOT MERGE release-1.3.1 labels Aug 7, 2018

rvl changed the title ~~DEVOPS-834 Cardano faucet package~~ [DEVOPS-834] Cardano faucet package Aug 7, 2018

rvl changed the base branch from release/1.3.0 to release/1.3.1 August 9, 2018 13:54

rvl removed the DO NOT MERGE label Aug 9, 2018

rvl merged commit 324fbc2 into release/1.3.1 Aug 10, 2018

rvl deleted the faucet branch August 10, 2018 08:52

commandodev mentioned this pull request Sep 5, 2018

Faucet rebased and squashed against develop #3541

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DEVOPS-834] Cardano faucet package #2939

[DEVOPS-834] Cardano faucet package #2939

commandodev commented May 16, 2018 •

edited by rvl

Loading

cleverca22 May 22, 2018

cleverca22 May 22, 2018

cleverca22 May 22, 2018

commandodev May 22, 2018

cleverca22 left a comment

erikd Jul 14, 2018

commandodev Jul 16, 2018

erikd Jul 14, 2018

rvl left a comment •

edited

Loading

commandodev commented Jul 20, 2018 •

edited

Loading

commandodev commented Jul 20, 2018 •

edited

Loading

rvl Jul 23, 2018

commandodev Jul 23, 2018

rvl Jul 23, 2018

rvl commented Jul 24, 2018

rvl commented Jul 25, 2018 •

edited

Loading

rvl commented Aug 7, 2018

		@@ -0,0 +1 @@
		{"wallet-id":"Ae2tdPwUPEZJkLTchA9mETtyXumBT1K1gqtX3dgXUguJFLuvyNU9BzQXrEK","recovery-words":["relax","secret","seek","story","supreme","meat","judge","onion","weasel","sponsor","garlic","erase"],"address":"DdzFFzCqrhswmBoqDubnsZZpvnfFbjgwLwFyk4vC5jJJf75m36f4M3f67k4iia1RQdCUPmJHV7c9TK2ogfzjoQSYHKCoM4YTv6zrmr28","account-index":2147483648}

[DEVOPS-834] Cardano faucet package #2939

[DEVOPS-834] Cardano faucet package #2939

Conversation

commandodev commented May 16, 2018 • edited by rvl Loading

Description

Linked issue

Type of change

Developer checklist

Testing checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cleverca22 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvl left a comment • edited Loading

Choose a reason for hiding this comment

commandodev commented Jul 20, 2018 • edited Loading

commandodev commented Jul 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvl commented Jul 24, 2018

rvl commented Jul 25, 2018 • edited Loading

rvl commented Aug 7, 2018

commandodev commented May 16, 2018 •

edited by rvl

Loading

rvl left a comment •

edited

Loading

commandodev commented Jul 20, 2018 •

edited

Loading

commandodev commented Jul 20, 2018 •

edited

Loading

rvl commented Jul 25, 2018 •

edited

Loading