feat: makes ip address of request available in application layer #2939

konstantinabl · 2024-09-04T07:40:22Z

Description:

Makes the ip of the sender of a request, available in the application layer. Also makes requestIdPrefix required, since we need to always have it for logging purposes

Related issue(s):

Fixes #2867
Fixes #2940

github-actions · 2024-09-04T12:14:00Z

Acceptance Tests

0 tests 0 ✔️ 0s ⏱️
0 suites 0 💤
0 files 0 ❌

Results for commit c7e1ea1.

♻️ This comment has been updated with latest results.

github-actions · 2024-09-09T17:07:57Z

Tests

      3 files   272 suites 19s ⏱️
1 276 tests 1 275 ✔️ 1 💤 0 ❌
1 288 runs 1 287 ✔️ 1 💤 0 ❌

Results for commit 4289cd3.

♻️ This comment has been updated with latest results.

packages/relay/src/lib/poller.ts

packages/relay/src/index.ts

packages/relay/src/lib/clients/mirrorNodeClient.ts

packages/relay/src/lib/types/IRequestDetails.ts

Nana-EC

Big change.
Left some thoughts

Nana-EC · 2024-09-18T20:23:45Z

packages/relay/src/lib/env.txt

@@ -0,0 +1,150 @@
+apiVersion: v1


Q: please revert this configmap file addition

Nana-EC · 2024-09-18T20:26:46Z

packages/server/src/koaJsonRpc/index.ts

@@ -106,6 +109,7 @@ export default class KoaJsonRpc {
  rpcApp(): (ctx: Koa.Context, _next: Koa.Next) => Promise<void> {
    return async (ctx: Koa.Context, _next: Koa.Next) => {
      this.requestId = ctx.state.reqId;
+      this.requestIpAddress = ctx.request.ip;


Q: should we only set this if the relay operator plans to rate limit by IP?

@Nana-EC the idea here is that we need this for the hbar limiter and spending plan, so we always want to have the sender's ip in order to check if its a part of the spending plan of a partner. so its not related to rate limiting but hbar limiting and we need it passed to sdkClient and the functions there

Nana-EC · 2024-09-18T20:27:31Z

packages/server/src/server.ts

+  const requestDetails = app.getRequestDetails();
+  logger.debug('Method name: ' + methodName);
+  logger.debug('Request id: ' + requestDetails.requestId);
+  logger.debug('Request ip: ' + requestDetails.ipAddress);


drop this line for now

Nana-EC · 2024-09-18T20:28:49Z

packages/server/src/server.ts

-  const requestIdPrefix = requestId ? formatRequestIdMessage(requestId) : '';
+const logAndHandleResponse = async (methodName: string, methodParams: any[], methodFunction: any) => {
+  const requestDetails = app.getRequestDetails();
+  logger.debug('Method name: ' + methodName);


Squash this into 1 line if needed.
That being said do we need it?
We should have a log that covers the method name no as well as the result

yeah this was needed for debugging purposes only. will remove it

Signed-off-by: Konstantina Blazhukova <[email protected]>

Signed-off-by: Victor Yanev <[email protected]>

…ingPlanRepository.spec.ts Signed-off-by: Victor Yanev <[email protected]>

Signed-off-by: Konstantina Blazhukova <[email protected]>

Signed-off-by: Victor Yanev <[email protected]>

Signed-off-by: Konstantina Blazhukova <[email protected]>

sonarcloud · 2024-09-20T16:11:36Z

Quality Gate failed

Failed conditions
6.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

codecov · 2024-09-20T16:21:39Z

Codecov Report

Attention: Patch coverage is 74.22907% with 117 lines in your changes missing coverage. Please review.

Project coverage is 78.94%. Comparing base (acd8f4a) to head (c7e1ea1).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/server/src/server.ts	1.19%	83 Missing ⚠️
packages/relay/src/lib/clients/sdkClient.ts	52.00%	12 Missing ⚠️
packages/relay/src/lib/eth.ts	93.24%	10 Missing ⚠️
packages/relay/src/lib/clients/mirrorNodeClient.ts	91.17%	3 Missing ⚠️
packages/server/src/koaJsonRpc/index.ts	40.00%	3 Missing ⚠️
packages/relay/src/lib/poller.ts	60.00%	2 Missing ⚠️
...kages/relay/src/lib/clients/cache/localLRUCache.ts	83.33%	1 Missing ⚠️
packages/relay/src/lib/relay.ts	66.66%	1 Missing ⚠️
.../lib/services/ethService/ethFilterService/index.ts	91.66%	1 Missing ⚠️
...ay/src/lib/services/metricService/metricService.ts	66.66%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2939       +/-   ##
===========================================
- Coverage   90.14%   78.94%   -11.20%     
===========================================
  Files          42       60       +18     
  Lines        3185     4027      +842     
  Branches      641      805      +164     
===========================================
+ Hits         2871     3179      +308     
- Misses        270      808      +538     
+ Partials       44       40        -4

Flag	Coverage Δ
	`23.61% <5.26%> (?)`
relay	`90.15% <91.50%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/relay/src/formatters.ts	`41.78% <ø> (ø)`
packages/relay/src/index.ts	`100.00% <ø> (ø)`
packages/relay/src/lib/clients/cache/redisCache.ts	`92.39% <100.00%> (ø)`
...barLimiter/ethAddressHbarSpendingPlanRepository.ts	`100.00% <100.00%> (ø)`
...sitories/hbarLimiter/hbarSpendingPlanRepository.ts	`100.00% <100.00%> (ø)`
...hbarLimiter/ipAddressHbarSpendingPlanRepository.ts	`100.00% <100.00%> (ø)`
packages/relay/src/lib/hbarlimiter/index.ts	`100.00% <100.00%> (ø)`
packages/relay/src/lib/precheck.ts	`94.79% <100.00%> (-0.36%)`	⬇️
...elay/src/lib/services/cacheService/cacheService.ts	`95.48% <100.00%> (ø)`
...kages/relay/src/lib/services/debugService/index.ts	`80.88% <100.00%> (ø)`
... and 15 more

... and 15 files with indirect coverage changes

konstantinabl linked an issue Sep 4, 2024 that may be closed by this pull request

[HBAR Rate Limit Redesign] Make the ip address of the request available in the application layer #2867

Open

konstantinabl force-pushed the 2867-make-the-ip-address-of-the-request-available-in-the-application-layer branch from b81e167 to e946dbf Compare September 4, 2024 12:10

konstantinabl force-pushed the 2867-make-the-ip-address-of-the-request-available-in-the-application-layer branch 2 times, most recently from fbaf5ca to 2bcdac1 Compare September 9, 2024 13:45

konstantinabl changed the title ~~Makes ip address of request available in application layer~~ feat: makes ip address of request available in application layer Sep 9, 2024

konstantinabl added the enhancement New feature or request label Sep 9, 2024

konstantinabl added this to the 0.56.0 milestone Sep 9, 2024

konstantinabl force-pushed the 2867-make-the-ip-address-of-the-request-available-in-the-application-layer branch 2 times, most recently from dc0bb30 to 2b4d451 Compare September 9, 2024 17:04

victor-yanev requested changes Sep 10, 2024

View reviewed changes

konstantinabl force-pushed the 2867-make-the-ip-address-of-the-request-available-in-the-application-layer branch 3 times, most recently from 2b1d1d0 to e16946e Compare September 11, 2024 10:22

konstantinabl self-assigned this Sep 11, 2024

konstantinabl force-pushed the 2867-make-the-ip-address-of-the-request-available-in-the-application-layer branch 4 times, most recently from 0f4c413 to fe08a97 Compare September 16, 2024 13:18

victor-yanev mentioned this pull request Sep 16, 2024

feat: Implement resetLimit #2966

Merged

2 tasks

konstantinabl force-pushed the 2867-make-the-ip-address-of-the-request-available-in-the-application-layer branch 4 times, most recently from e4a641d to dd3f2d4 Compare September 17, 2024 10:24

quiet-node modified the milestones: 0.56.0, 0.57.0 Sep 17, 2024

Nana-EC reviewed Sep 18, 2024

View reviewed changes

konstantinabl added 2 commits September 19, 2024 23:43

Makes ip address of request available in application layer

4c0869e

Signed-off-by: Konstantina Blazhukova <[email protected]>

Fixes failing unit tests

4fb59a3

Signed-off-by: Konstantina Blazhukova <[email protected]>

victor-yanev and others added 18 commits September 20, 2024 13:53

chore: fix build

ee2dc12

Signed-off-by: Victor Yanev <[email protected]>

chore: fix ethAddressHbarSpendingPlanRepository.spec.ts and hbarSpend…

0c1a2bc

…ingPlanRepository.spec.ts Signed-off-by: Victor Yanev <[email protected]>

Fixes acceptance tests using request details

b75e1f7

Signed-off-by: Konstantina Blazhukova <[email protected]>

Fixes ws-server acceptance tests

ac74abd

Signed-off-by: Konstantina Blazhukova <[email protected]>

Adds better typing in accpetance tests as well as fixing them

c8f042d

Signed-off-by: Konstantina Blazhukova <[email protected]>

temporary fix

d2f99c7

Signed-off-by: Konstantina Blazhukova <[email protected]>

fix: build error

e48d7ec

Signed-off-by: Victor Yanev <[email protected]>

fix: ws errors

ef6e2c0

Signed-off-by: Victor Yanev <[email protected]>

fix: metricService.spec.ts

6ee4195

Signed-off-by: Victor Yanev <[email protected]>

fix: mirrorNodeClient

34be5ae

Signed-off-by: Victor Yanev <[email protected]>

fix: validators.ts

88f85ce

Signed-off-by: Victor Yanev <[email protected]>

fix: hapiService.spec.ts

90ad401

Signed-off-by: Victor Yanev <[email protected]>

fix: precheck.spec.ts and hbarSpendingPlanRepository.spec.ts

42ed1a8

Signed-off-by: Victor Yanev <[email protected]>

fixes hbar calculation in rpc batch1 test

e787c32

Signed-off-by: Konstantina Blazhukova <[email protected]>

Fixes precompile test

2ebff41

Signed-off-by: Konstantina Blazhukova <[email protected]>

Fixes unit tests after adding requestDetails as required

66aeeb6

Signed-off-by: Konstantina Blazhukova <[email protected]>

Fixes failing tests in filter accetance test

5b795b7

Signed-off-by: Konstantina Blazhukova <[email protected]>

Fixes build after main merge

633e385

Signed-off-by: Konstantina Blazhukova <[email protected]>

konstantinabl force-pushed the 2867-make-the-ip-address-of-the-request-available-in-the-application-layer branch from 1f9c448 to 633e385 Compare September 20, 2024 11:37

konstantinabl added 2 commits September 20, 2024 18:34

merge main and fix tests

b8b0682

Signed-off-by: Konstantina Blazhukova <[email protected]>

Addresses PR comments

c7e1ea1

Signed-off-by: Konstantina Blazhukova <[email protected]>

konstantinabl marked this pull request as ready for review September 20, 2024 16:10

konstantinabl requested review from AlfredoG87, ebadiere, georgi-l95, Ivo-Yankov, a team and lukelee-sl as code owners September 20, 2024 16:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: makes ip address of request available in application layer #2939

feat: makes ip address of request available in application layer #2939

konstantinabl commented Sep 4, 2024 •

edited by quiet-node

Loading

github-actions bot commented Sep 4, 2024 •

edited

Loading

github-actions bot commented Sep 9, 2024 •

edited

Loading

Nana-EC left a comment •

edited

Loading

Nana-EC Sep 18, 2024

Nana-EC Sep 18, 2024

konstantinabl Sep 20, 2024

Nana-EC Sep 18, 2024

Nana-EC Sep 18, 2024

konstantinabl Sep 20, 2024

sonarcloud bot commented Sep 20, 2024

codecov bot commented Sep 20, 2024

feat: makes ip address of request available in application layer #2939

Are you sure you want to change the base?

feat: makes ip address of request available in application layer #2939

Conversation

konstantinabl commented Sep 4, 2024 • edited by quiet-node Loading

github-actions bot commented Sep 4, 2024 • edited Loading

Acceptance Tests

github-actions bot commented Sep 9, 2024 • edited Loading

Tests

Nana-EC left a comment • edited Loading

Choose a reason for hiding this comment

Nana-EC Sep 18, 2024

Choose a reason for hiding this comment

Nana-EC Sep 18, 2024

Choose a reason for hiding this comment

konstantinabl Sep 20, 2024

Choose a reason for hiding this comment

Nana-EC Sep 18, 2024

Choose a reason for hiding this comment

Nana-EC Sep 18, 2024

Choose a reason for hiding this comment

konstantinabl Sep 20, 2024

Choose a reason for hiding this comment

sonarcloud bot commented Sep 20, 2024

Quality Gate failed

codecov bot commented Sep 20, 2024

Codecov Report

konstantinabl commented Sep 4, 2024 •

edited by quiet-node

Loading

github-actions bot commented Sep 4, 2024 •

edited

Loading

github-actions bot commented Sep 9, 2024 •

edited

Loading

Nana-EC left a comment •

edited

Loading