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

fix: reduce hbar limit only by operator fees #2755

Conversation

Ivo-Yankov
Copy link
Collaborator

@Ivo-Yankov Ivo-Yankov commented Jul 25, 2024

Description:
This PR separates the transaction fee from the gas fee paid by the relay operator and the signer. Reduces the Hbar limiter only by the amount paid by the operator. This is done by traversing transactionRecord.transfers for the submitted EthereumTransaction.

Also adds a test that checks how much was the Hbar Limiter reduced after a large transaction, how much the operator has spent, the sum of all fees related to that transaction and compares those values. The test passes if the values are within 1% tolerance range from one another. The tolerated differance comes from fees from .getRecord, .getReceipt, etc.

Related issue(s):

Fixes #2729 #2691

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@Ivo-Yankov Ivo-Yankov self-assigned this Jul 25, 2024
@Ivo-Yankov Ivo-Yankov added the enhancement New feature or request label Jul 25, 2024
@Ivo-Yankov Ivo-Yankov added this to the 0.53.0 milestone Jul 25, 2024
Copy link

github-actions bot commented Jul 25, 2024

Tests

    2 files  158 suites   13s ⏱️
870 tests 869 ✔️ 1 💤 0
883 runs  882 ✔️ 1 💤 0

Results for commit 36ed06d.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jul 25, 2024

Acceptance Tests

  22 files  244 suites   31m 33s ⏱️
615 tests 607 ✔️ 4 💤 4
702 runs  693 ✔️ 4 💤 5

Results for commit 36ed06d.

♻️ This comment has been updated with latest results.

Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
@Ivo-Yankov Ivo-Yankov marked this pull request as ready for review July 25, 2024 20:29
Copy link
Contributor

@victor-yanev victor-yanev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG, I've left a few suggestions

packages/server/tests/helpers/assertions.ts Outdated Show resolved Hide resolved
packages/server/tests/acceptance/hbarLimiter.spec.ts Outdated Show resolved Hide resolved
packages/server/tests/acceptance/hbarLimiter.spec.ts Outdated Show resolved Hide resolved
packages/server/tests/acceptance/hbarLimiter.spec.ts Outdated Show resolved Hide resolved
packages/server/tests/acceptance/hbarLimiter.spec.ts Outdated Show resolved Hide resolved
packages/server/tests/acceptance/hbarLimiter.spec.ts Outdated Show resolved Hide resolved
packages/relay/src/formatters.ts Outdated Show resolved Hide resolved
packages/relay/src/lib/clients/sdkClient.ts Outdated Show resolved Hide resolved
packages/relay/src/lib/clients/sdkClient.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@ebadiere ebadiere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG. One question.

victor-yanev
victor-yanev previously approved these changes Jul 30, 2024
Copy link
Contributor

@victor-yanev victor-yanev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

quiet-node
quiet-node previously approved these changes Jul 30, 2024
Copy link
Member

@quiet-node quiet-node left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! LGTM!

packages/server/tests/acceptance/hbarLimiter.spec.ts Outdated Show resolved Hide resolved
@Ivo-Yankov Ivo-Yankov dismissed stale reviews from victor-yanev and quiet-node via 54ed9b9 July 31, 2024 13:23
Signed-off-by: Ivo Yankov <[email protected]>
…-verify-transaction-costs-and-metrics

Signed-off-by: Eric Badiere <[email protected]>
quiet-node
quiet-node previously approved these changes Jul 31, 2024
victor-yanev
victor-yanev previously approved these changes Jul 31, 2024
acuarica
acuarica previously approved these changes Jul 31, 2024
Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lg

Copy link

sonarcloud bot commented Aug 1, 2024

@ebadiere ebadiere merged commit 3dc8b4f into main Aug 1, 2024
35 of 36 checks passed
@ebadiere ebadiere deleted the 2691-add-tests-around-the-sdkclient-class-to-verify-transaction-costs-and-metrics branch August 1, 2024 14:06
ebadiere added a commit that referenced this pull request Aug 1, 2024
* fix: reduce hbar limit only by operator fees

Signed-off-by: Ivo Yankov <[email protected]>

* chore: update expected values

Signed-off-by: Ivo Yankov <[email protected]>

* chore: fix code smells

Signed-off-by: Ivo Yankov <[email protected]>

* nit: re-trigger ci

Signed-off-by: Ivo Yankov <[email protected]>

* chore: addressing comments

Signed-off-by: Ivo Yankov <[email protected]>

* fix: hbar limit error handle

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* nit: update comment

Signed-off-by: Ivo Yankov <[email protected]>

* fix: unit test mocks

Signed-off-by: Ivo Yankov <[email protected]>

* fix: update assertions and mocks

Signed-off-by: Ivo Yankov <[email protected]>

---------

Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>
Co-authored-by: Eric Badiere <[email protected]>
@ebadiere ebadiere mentioned this pull request Aug 1, 2024
2 tasks
ebadiere added a commit that referenced this pull request Aug 1, 2024
* fix: reduce hbar limit only by operator fees

Signed-off-by: Ivo Yankov <[email protected]>

* chore: update expected values

Signed-off-by: Ivo Yankov <[email protected]>

* chore: fix code smells

Signed-off-by: Ivo Yankov <[email protected]>

* nit: re-trigger ci

Signed-off-by: Ivo Yankov <[email protected]>

* chore: addressing comments

Signed-off-by: Ivo Yankov <[email protected]>

* fix: hbar limit error handle

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* nit: update comment

Signed-off-by: Ivo Yankov <[email protected]>

* fix: unit test mocks

Signed-off-by: Ivo Yankov <[email protected]>

* fix: update assertions and mocks

Signed-off-by: Ivo Yankov <[email protected]>

---------

Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>
Co-authored-by: Eric Badiere <[email protected]>
Signed-off-by: ebadiere <[email protected]>
ebadiere added a commit that referenced this pull request Aug 1, 2024
* fix: reduce hbar limit only by operator fees

Signed-off-by: Ivo Yankov <[email protected]>

* chore: update expected values

Signed-off-by: Ivo Yankov <[email protected]>

* chore: fix code smells

Signed-off-by: Ivo Yankov <[email protected]>

* nit: re-trigger ci

Signed-off-by: Ivo Yankov <[email protected]>

* chore: addressing comments

Signed-off-by: Ivo Yankov <[email protected]>

* fix: hbar limit error handle

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* nit: update comment

Signed-off-by: Ivo Yankov <[email protected]>

* fix: unit test mocks

Signed-off-by: Ivo Yankov <[email protected]>

* fix: update assertions and mocks

Signed-off-by: Ivo Yankov <[email protected]>

---------

Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>
Co-authored-by: Eric Badiere <[email protected]>
@ebadiere ebadiere mentioned this pull request Aug 1, 2024
2 tasks
ebadiere added a commit that referenced this pull request Aug 1, 2024
* fix: reduce hbar limit only by operator fees

Signed-off-by: Ivo Yankov <[email protected]>

* chore: update expected values

Signed-off-by: Ivo Yankov <[email protected]>

* chore: fix code smells

Signed-off-by: Ivo Yankov <[email protected]>

* nit: re-trigger ci

Signed-off-by: Ivo Yankov <[email protected]>

* chore: addressing comments

Signed-off-by: Ivo Yankov <[email protected]>

* fix: hbar limit error handle

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* nit: update comment

Signed-off-by: Ivo Yankov <[email protected]>

* fix: unit test mocks

Signed-off-by: Ivo Yankov <[email protected]>

* fix: update assertions and mocks

Signed-off-by: Ivo Yankov <[email protected]>

---------

Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>
Co-authored-by: Eric Badiere <[email protected]>
ebadiere added a commit that referenced this pull request Aug 1, 2024
* fix: reduce hbar limit only by operator fees

Signed-off-by: Ivo Yankov <[email protected]>

* chore: update expected values

Signed-off-by: Ivo Yankov <[email protected]>

* chore: fix code smells

Signed-off-by: Ivo Yankov <[email protected]>

* nit: re-trigger ci

Signed-off-by: Ivo Yankov <[email protected]>

* chore: addressing comments

Signed-off-by: Ivo Yankov <[email protected]>

* fix: hbar limit error handle

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* nit: update comment

Signed-off-by: Ivo Yankov <[email protected]>

* fix: unit test mocks

Signed-off-by: Ivo Yankov <[email protected]>

* fix: update assertions and mocks

Signed-off-by: Ivo Yankov <[email protected]>

---------

Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>
Co-authored-by: Eric Badiere <[email protected]>
Signed-off-by: ebadiere <[email protected]>
ebadiere added a commit that referenced this pull request Aug 1, 2024
* test: added E2E test for preemtive rate limit (#2753)

* test: added E2E test fir preemtive rate limit

Signed-off-by: Logan Nguyen <[email protected]>

* Update hbarLimiter.spec.ts

Signed-off-by: Logan Nguyen <[email protected]>

---------

Signed-off-by: Logan Nguyen <[email protected]>
Signed-off-by: ebadiere <[email protected]>

* test: expand integration tests for SDK client (#2756)

* test: expand integration tests for SDK client

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

* chore: minor improvements to sdkClient.ts

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

* fix: typescript error

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

* fix: simplify test data

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

* chore: formatting

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

* chore: fix typescript errors and warnings

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

* chore: optimize imports

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

* chore: address comments

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

* chore: improve readability of tests

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

* test: fix wrong MIRROR_NODE_LIMIT_PARAM in relay tests

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

* fix: wrong env file loaded

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

* test: remove hardcoded usages of MIRROR_NODE_LIMIT_PARAM in relay tests

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

* test: revert changes to rpc_batch1.spec.ts

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

* chore: remove unused import

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

* chore: fix test after merge

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

* chore: fix test after merge

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

* chore: revert changes to usages of MIRROR_NODE_LIMIT_PARAM

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

* chore: revert changes to usages of MIRROR_NODE_LIMIT_PARAM

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

---------

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

* feat: search contract logs only by transaction hash (#2762)

* chore: refactor eth.ts

Signed-off-by: nikolay <[email protected]>

* chore: bump hedera-local version

Signed-off-by: nikolay <[email protected]>

---------

Signed-off-by: nikolay <[email protected]>
Signed-off-by: ebadiere <[email protected]>

* chore: Implement memory leak detection in tests (#2695)

* chore: Implement memory leak detection in tests

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

* chore: Implement memory leak detection in tests

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

* chore: Implement memory leak detection in tests

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

* chore: Implement memory leak detection in tests

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

* chore: Implement memory leak detection in tests

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

* chore: add docs

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

* chore: fix sonar issues

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

* chore: formatting

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

* chore: small fix

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

* chore: trace GC and write snapshot if memory leak > 0.5 MB

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

* chore: revert false changes to eth.ts

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

* chore: optimize code

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

* Merge branch 'main' into 2260-Implement-a-Memory-Leak-detection-Test

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

# Conflicts:
#	package-lock.json
#	package.json
#	packages/server/tsconfig.json

* fix: Do not write heap snapshots for acceptance tests

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

* fix: add TODO for removing --trace_gc flag

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

* test: add github comment in cases a test is having memory leaks

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

* test: add github context env variables

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

* test: extract common logic for github api calls

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

* test: console.debug -> console.log for successful PR comments

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

* test: fix request to github API

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

* Merge branch 'main' into 2260-Implement-a-Memory-Leak-detection-Test

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

# Conflicts:
#	packages/server/tests/acceptance/index.spec.ts

* Merge branch 'main' into 2260-Implement-a-Memory-Leak-detection-Test

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

# Conflicts:
#	packages/server/tests/acceptance/index.spec.ts

* fix: request to github API

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

* chore: add upload heap snapshots step in github workflow for acceptance and integration tests

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

* fix: add `subject_type: file` to github request for adding comment on PR

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

* fix: fix path in request to github API

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

* chore: extract github request logic in new file - githubClient.ts

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

* fix: path of upload-artifact action for uploading heap snapshots

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

* fix: requests in githubClient.ts

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

* fix: requests in githubClient.ts

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

* fix: format message of github-actions[bot]

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

* fix: format message of github-actions[bot]

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

* chore: remove unused github context env variable

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

* chore: formatting

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

* chore: formatting

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

* chore: formatting

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

* fix: `formatBytes` returns `NaN undefined` when `0` is passed to it

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

* fix: `formatBytes` returns `NaN undefined` when negative number is passed to it

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

* fix: predicate for updating existing comment

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

* fix: remove unused constant + temporary console logs

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

* fix: make memory leak report more descriptive

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

* chore: remove unnecessary console logs

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

* chore: reduce MEMORY_LEAK_SNAPSHOT_THRESHOLD to 500 KB

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

* fix: update logic for detecting memory leak based on heap size threshold

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

* fix: add separate threshold for taking heap snapshots

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

* fix: set `WRITE_SNAPSHOT_ON_MEMORY_LEAK` to be `false` by default

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

* test: add warm-up phase to memory leak detection

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

* chore: revert changes to rpc_batch1.spec.ts

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

---------

Signed-off-by: Victor Yanev <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>
Co-authored-by: Eric Badiere <[email protected]>
Signed-off-by: ebadiere <[email protected]>

* feat: deprecate `ETH_POPULATE_SYNTHETIC_CONTRACT_RESULTS` env (#2768)

* chore: remove env

Signed-off-by: nikolay <[email protected]>

* chore: modify test

Signed-off-by: nikolay <[email protected]>

---------

Signed-off-by: nikolay <[email protected]>
Signed-off-by: ebadiere <[email protected]>

* Bump versions for v0.53.0-rc1

Signed-off-by: Swirlds Automation <[email protected]>
Signed-off-by: ebadiere <[email protected]>

* feat: reused executetransaction for methods in sdkclient and improved… (#2776)

feat: reused executetransaction for methods in sdkclient and improved logging (#2749)

* fix: reused executeTransaction() in deleteFile() method

* fix: reworked executeTransaction() to accept executeAll

* feat: fix: reused executeTransaction() in fileCreate() method

* fix: removed check rate limit in executeGetTransactionRecord

* fix: fixed SDK HBAR limiter tests

* fix: reverted rework executeTransaction()

* fix: reworked formatRequestIdMessage

* feat: added executeAllTransaction() method

* fix: made shouldLimit optional in executeTransaction()

* Update packages/relay/src/lib/clients/sdkClient.ts

* fix: addressed comments

* fix: extracted handleExecuteAllError()

* fix: renamed handleExecuteAllError to getTransactionMetrics and reuse it in executeTransaction

---------

Signed-off-by: Logan Nguyen <[email protected]>
Signed-off-by: Logan Nguyen <[email protected]>
Co-authored-by: Logan Nguyen <[email protected]>
Co-authored-by: Victor Yanev <[email protected]>
Signed-off-by: ebadiere <[email protected]>

* Bump versions for v0.53.0-rc2

Signed-off-by: Swirlds Automation <[email protected]>
Signed-off-by: ebadiere <[email protected]>

* fix: reduce hbar limit only by operator fees (#2755)

* fix: reduce hbar limit only by operator fees

Signed-off-by: Ivo Yankov <[email protected]>

* chore: update expected values

Signed-off-by: Ivo Yankov <[email protected]>

* chore: fix code smells

Signed-off-by: Ivo Yankov <[email protected]>

* nit: re-trigger ci

Signed-off-by: Ivo Yankov <[email protected]>

* chore: addressing comments

Signed-off-by: Ivo Yankov <[email protected]>

* fix: hbar limit error handle

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* fix: reduce test flakiness

Signed-off-by: Ivo Yankov <[email protected]>

* nit: update comment

Signed-off-by: Ivo Yankov <[email protected]>

* fix: unit test mocks

Signed-off-by: Ivo Yankov <[email protected]>

* fix: update assertions and mocks

Signed-off-by: Ivo Yankov <[email protected]>

---------

Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>
Co-authored-by: Eric Badiere <[email protected]>
Signed-off-by: ebadiere <[email protected]>

---------

Signed-off-by: Logan Nguyen <[email protected]>
Signed-off-by: ebadiere <[email protected]>
Signed-off-by: Victor Yanev <[email protected]>
Signed-off-by: nikolay <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>
Signed-off-by: Swirlds Automation <[email protected]>
Signed-off-by: Logan Nguyen <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Co-authored-by: Logan Nguyen <[email protected]>
Co-authored-by: Victor Yanev <[email protected]>
Co-authored-by: Nikolay Atanasow <[email protected]>
Co-authored-by: Swirlds Automation <[email protected]>
Co-authored-by: Ivo Yankov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
5 participants