-
Notifications
You must be signed in to change notification settings - Fork 0
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
Set up IaC for deployments #6
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes more space to locate infra inside the `web-app` folder. The paths were renamed as necessary
- We wrapped the remix app in a lambda handler and build it with esbuild. - The path to the static build assets was also modified in the remix config. Additionally redirects were setup for the favicon to the static path, via the express handler. This `/static/*` route will eventually be handled by CloudFront + S3
We make sure the building of assets in the `src` folder also happens before trying to deploy
We needed to set up @typescript-eslint for parsing, as well as @Stylistic for opinionated styles
By doing so we can import the env schemas from other projects. In order to support the same, we needed to change the build process for the infra repo. This involved getting substituting `ts-node` with `tsx`. As a side task, the build and watch scripts were removed from package.json as they are [redundant](aws/aws-cdk#19516 (comment))
- A `deploy-prod` script was added, which deploys with an address stored in cast wallet, and securely accessed via interactive shell. The script also verifies the contract code on etherscan - The deployment script `01_Deploy.s.sol` was modified to use the deployer address provided via forge flags, instead of reading from the env. This is safer. - _.env_ file was modified to include all the required safe variables - We deployed this code to Sepolia test net! So you will also see the logs for this deploy run, inside _contracts/broadcast/*_
When the user rejects a staking/unstaking transaction from metamask while the modal is open, it causes the user data to refresh. While the data is refreshing, the modal is empty. This is unwanted behaviour. To fix this, we close the modal automatically when user data is loading. The loading spinner is anyway shown on the card (on Staked Amount number) when user data is loading, and the Stake/Unstake buttons are disabled.
…files A cyclical dependency was being created, as the zod schema for dappInfo/chainInfo is used in the `process.env` parser, but the `process.env` parser is used in the dappInfo/chainInfo getter. The problem was easily solved by moving the schema and the getter into their own dedicated files. All imports to these were also updated
Now we can access browser env variables from anywhere on the client, using `getBrowserEnv`. This eliminates the need to pass the data from loader all the way through several layers of abstraction. We can directly read the env data which has been obtained from the root loader, using `getBrowserEnv`. A dedicated component `BrowserEnv` is used to set this up
Earlier, the chainInfo and dappInfo were passed in as parameters to the RTK query layer. This is problematic in the following ways: - These parameters, while required information, are not pertinent to the request/endpoint in question. Therefore, it seems odd that we are passing this in as information. - We had to read the root loader data at multiple levels of the component tree. While this doesn't incur multiple requests (loaders only run once, regardless of how many times you read their data), the data in question (dappInfo, chainInfo) actually resembles environment variables, and it makes sense to have them available globally, and not have to fetch them explicitly. - This is a lot of repeated code We fixed this, by removing said params, and instead reading the chainInfo & dappInfo from the parsed browser env. We do this directly in the `FinthetixStakingContractHandler`, which removes a lot of repeated code. The metamask handler generation was also moved into the `FinthetixStakingContractHandler.make` function, thereby removing even more repeated code
… to `getServerEnv` - Adding the block explorer info to .env allows us to use different block explorer URLs as per requirement. The URL is used directly from `getBrowserEnv` (essentially `window.ENV`), inside metamask handlers, and passed through `useRootLoaderData` in components where the data flow is clearer and there are SSR hydration concerns. - `getParsedEnv` was renamed to `getServerEnv`. This makes it more clear, the distinction from `getBrowserEnv`.
Earlier, we only showed a success message when user requested sample tokens. Now we show the link to the transaction on block explorer as well.
Earlier we passed the flags via CLI. It is better to pass it using ESBuild's recommended [script config](https://esbuild.github.io/getting-started/#build-scripts).
- We can use this to run the production server locally, to see if everything works. The key differences between this and the development server are - The code is in a bundled state. Plus, optimum performance - We use the chainInfo provided from .env, instead of a local chain. So you can add the production chain info, and test the server locally - To run the server locally, we added a `server/index.production.local.ts` file, which simply starts the express server on port 3000. - We also handled the static path hosting (at `/static/*`) via `express.static`. This will be unused in production, as we will configure Cloudfront to forward requests to s3. However, in local this is helpful. - Instead of bundling the production server, we run it on the fly with `tsx` - The schema for `NODE_ENV` was modified to use enums instead of a union of literals. - A bug in the production server was fixed, which was preventing the Remix handler from correctly handling requests. Essentially, `app.use` was converted to `app.all`
This route captures all invalid route requests. We also made the _Finthetix_ logo a link, except on dashboard page
- When users are logged into the dapp, but their metamask is in a different chain, this causes issues with the dapp. So we created a Dialog prompt, which asks the user to either switch to the correct chain manually (allowing them to use their custom RPC server) or programmatically by clicking our button (and using the RPC server we provide). `ChainSwitcher` component encapsulates all this logic. - The current/active Chain ID is now stored in the global state user slice. Actions, reducers, types and selectors were added as necessary - In order to find out the active chain ID, we interface with metamask. This is done in two ways. - First through the `eth_chainId` method, used to get the active chain when user first logs in. A wrapper method `getActiveChainId` was added on `MetamaskHandler` class, for this purpose - Second, through the `chainChanged` event handler, in order to detect when the user changes their chain while using the dapp post initial login. The types for this event were added to the global ethereum object, allowing us to setup and unmount event handlers in a typesafe way. - The dashboard page logic was modified to only start fetching data when the user is confirmed to be in the correct chain. As a result, the _Finthetix_ status query (cooldown time etc.) was made into a trigger based (RTK) query, instead of an automatic one. This is necessary as any interaction with the blockchain is over the RPC. So incorrect chain means that the RPC would search in a blockchain that doesn't have our required contracts. - The Dialog shown for switching chains is not meant to be dismissed, as this would render the dapp unusable. So we had to modify the Dialog UI component to include a custom prop `hideCloseBtn`. On enabling this prop (boolean), the close button is not rendered. The actual logic of preventing the modal from closing is handled by the dialog's parent component, by not handling the `onOpenChange` event. - The `UnderlineLink` component was modified to use the custom styles at the root level, rather than at the anchor tag level. This allows us to prevent the `ExternalLinkIcon` from wrapping around, by simply providing custom styles (`whitespace-nowrap`) - Some helpful comments were added to the cooldown-time-calculator custom hook
- We use useEffects to attempt auto-login with metamask address and to check the currently active chainId of the user on metamask. However if metamask is not installed, these result in an error. To prevent this, these metamask handler invocations were made to run only when user is already logged in. This makes sense currently, as the auto-login, and chain switching event listeners only make sense once the user is logged in. - The structure of the custom hooks used for this purpose was modified. They no longer depend on the key dependencies being passed in as parameters. Instead, these key dependencies, which are mostly global hooks, are directly used within the custom hooks for auto account-switch and auto chain-switch. - The selector for checking whether user is logged in was modified. It now only considers user to be logged in once the address is obtained from Metamask provider, not from localstorage. This makes sense, cause we don't want to setup the chain and account switch listeners before user explicitly interacts with metamask (by logging in). If they logged in previously, then uninstalled metamask, this is an error case, which should be caught by our auto-login functionality, and handled with a toast message. Such users are automatically logged out. So our application doesn't error when setting up the listeners (cause it never tries to)
- This way, when the active chain ID is fetched/synced, we automatically refetch the user and dapp data. This makes sure that all data is up to date, in case the previously used RPC was problematic - The label of `isUserInCorrectChain` was fixed in dashboard route component. The variable actually represents if the user is in the _wrong_ chain
Trying to use the network already added by the user, reduces a lot of friction and reduces the trust required to use the dapp. We first try to switch to user's RPC on the correct network. If they reject, that's that. Else if they have no RPC, we suggest our own. That way we also save on our RPC requests.
- Added title and meta tags for 404/splat page - Updated favicon
- We cached all routes and loaders that have public info, and also the favicon redirect done on the production server - The cache config is loaded in via .env, and parsed for typesafety. - A prefetch tag was added to navbar link
_Problem:_ When we deploy to lambda, we bundle the code. As a result, there is a bit of confusion regarding whether the code is running as an ESModule or as CommonJS modules. This also depends on the bundler we use. _Solution:_ - For production use(lambda), we avoid running the `import.meta.url` code. This is not necessary as on production the static asset requests are routed to s3 by cloudfront. However, locally we run this code to determine the relative path to the `public` folder where the static assets are hosted. - In order to conditionally run the `import.meta.url` code locally, we added a new optional env variable called `IS_RUNNING_LOCALLY`. Only if it is set to `true` (a string), we host the static assets, and thus run the `import.meta.url` code. This statement is handled easily by our local production server bundler (tsx).
We added a lambda + s3 + cloudfront setup to host the app. All the constructs were added. They were abstracted to custom classes as required. The env variables' schema is pulled in from the application server, to make sure we set all envs correctly. Any additional env variables required for deployment were added as necessary.
- Due to the nature of RPC servers, we are limited in the number of blocks queryable for data. To avoid errors in fetching logs, we had to scope the requests to the max number of blocks queryable by the RPC. - For this purpose, we added an env variable `RPC_QUERY_MAX_BLOCK_COUNT`, and added it to the schema as well. This is passed to the client via root loader data, and then subsequently set on the browser env as well. - On the client side, it is used via the root loader to show the number of blocks queried, on the Log Chart Data component. It's important to load this in via loader data instead of env, as we want to render on server as well. The chart component styles were modified to allow a heading for the card as well. - Also on the client side, we use this variable in our RTK query metamask endpoint, in order to scope the log request to max queryable block limit. - The Metamask Handler was modified to: - Accept block ranges in log data requests - Optionally accept the metamask handler instance as an argument, instead of making it everytime. This helps us reuse metamask handlers wherever possible. - The required env variables were also added to _web-app/infra/.env.sample_ - `STATIC_CACHE_TIME_IN_S` was also added to the _.env.sample_ as it was forgotten earlier.
This allows users to view the transaction details on block explorer, by clicking on the metamask notification. Our application anyway shows this as a toast. But metamask is more easily trusted by the user.
- This allows devs to easily understand what the env variable represents - Also added title to root error component
- We no longer need env variables to run tests. The deployment is now carried out by the default account. This is because we converted the production deployment script to use cast wallet instead of private keys passed through the .env - The README was updated to include more info about deploying to local and production chains
sayandcode
force-pushed
the
misc/iac-for-web-app
branch
from
March 29, 2024 07:09
a997cb7
to
5a882cf
Compare
sayandcode
force-pushed
the
misc/iac-for-web-app
branch
from
March 29, 2024 07:41
5a882cf
to
d92a48b
Compare
- We build, typecheck and lint. This makes sure everything still works at least according to on static checks - Scripts for building and typechecking were added to infra directories - Typechecking issues were fixed by modifying the tsconfig.json to ignore library typechecking, and also by fixing the type issues in our app files
sayandcode
force-pushed
the
misc/iac-for-web-app
branch
from
March 29, 2024 08:01
d92a48b
to
221fe60
Compare
The testing and web app static-checking pipelines were moved into a single github action file. This makes more logical sense. The jobs continue to run in parallel; but this is a more coherent positioning for the files
sayandcode
force-pushed
the
misc/iac-for-web-app
branch
from
March 29, 2024 08:12
e7c4ae1
to
9bfc0ee
Compare
- Node is now explicitly setup up with explicit versioning - AWS login is configured for the deployment step
sayandcode
force-pushed
the
misc/iac-for-web-app
branch
from
March 29, 2024 08:46
d5f963c
to
fb71138
Compare
We added the project tag on the resources via AWS CDK This allows us to identify the project when seeing the resource on AWS Management Web Console
sayandcode
force-pushed
the
misc/iac-for-web-app
branch
from
March 29, 2024 10:09
fb71138
to
f20b788
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR adds Infrastructure as Code to the web app.
Why?
This allows us to deploy the app repeatedly, in a reliable fashion.
How?
Contracts
Production Deployment script was added, and was used to deploy both to ETH Sepolia Testnet and Polygon Mumbai Testnet. We use Foundry's Cast Wallet to store and retrieve the private keys. The exact account to use is set via the env variables in the contracts directory.
Web App
The app is built for deploying on an AWS Lambda. The static assets are hosted on an S3 bucket. A Cloudfront CDN distribution was also added, for caching assets as well as the initial routing (static assets routed to s3, application to lambda)
Several issues in the app were also fixed as part of the PR, in order to make the application ready for production deployment. These fixes include:
Along with the above bugfixes, several tweaks were also made, such as:
Misc
Apart from the above, we also added better documentation (both in-code and dedicated), and added/streamlined the pipelines for both Quality Checking and Deployment.