feat: Adds draft design doc for new endpoints #3342

konstantinabl · 2024-12-18T17:39:18Z

Description:

Creates a design document for the new block explorer REST API endpoints

Related issue(s):

Fixes #3340

Notes for reviewer:

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

github-actions · 2024-12-18T17:44:46Z

Test Results

26 files + 2 382 suites +21 1h 15m 51s ⏱️ + 15m 31s
605 tests - 14 588 ✅ - 6 3 💤 - 1 14 ❌ - 7
954 runs +53 925 ✅ +55 8 💤 +4 21 ❌ - 6

For more details on these failures, see this check.

Results for commit e6f883c. ± Comparison against base commit 90739e1.

This pull request removes 18 and adds 4 tests. Note that renamed tests count towards both.

"after all" hook in "RPC Server Acceptance Tests" ‑ RPC Server Acceptance Tests "after all" hook in "RPC Server Acceptance Tests"
"after each" hook for "Subscribe to multiple contracts on same subscription" ‑ RPC Server Acceptance Tests Acceptance tests @web-socket-batch-3 eth_subscribe "after each" hook for "Subscribe to multiple contracts on same subscription"
"before all" hook for "emits an approval event" ‑ RPC Server Acceptance Tests Acceptance tests @erc20 Acceptance Tests HTS token should behave like erc20 transfer from when the token owner is not the zero address when the recipient is not the zero address when the spender has enough allowance "before all" hook for "emits an approval event"
"before all" hook for "reverts" ‑ RPC Server Acceptance Tests Acceptance tests @erc20 Acceptance Tests HTS token should behave like erc20 transfer from when the token owner is not the zero address when the recipient is not the zero address when the spender does not have enough allowance when the token owner has enough balance "before all" hook for "reverts"
"before each" hook for "reverts" ‑ RPC Server Acceptance Tests Acceptance tests @erc20 Acceptance Tests HTS token should behave like erc20 transfer from when the token owner is not the zero address when the recipient is not the zero address when the spender does not have enough allowance "before each" hook for "reverts"
"before each" hook for "reverts" ‑ RPC Server Acceptance Tests Acceptance tests @erc20 Acceptance Tests HTS token should behave like erc20 transfer from when the token owner is not the zero address when the recipient is the zero address "before each" hook for "reverts"
"before each" hook: reducing balance for "reverts" ‑ RPC Server Acceptance Tests Acceptance tests @erc20 Acceptance Tests HTS token should behave like erc20 transfer from when the token owner is not the zero address when the recipient is not the zero address when the spender does not have enough allowance when the token owner does not have enough balance "before each" hook: reducing balance for "reverts"
from/to Addresses in transaction between accounts are in evm format ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests Formats of addresses in Transaction and Receipt results from/to Addresses in transaction between accounts are in evm format
from/to Addresses in transaction to a contract (deployed through HAPI tx) are in evm and long-zero format ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests Formats of addresses in Transaction and Receipt results from/to Addresses in transaction to a contract (deployed through HAPI tx) are in evm and long-zero format
from/to Addresses in transaction to a contract (deployed through the relay) are in evm and long-zero format ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests Formats of addresses in Transaction and Receipt results from/to Addresses in transaction to a contract (deployed through the relay) are in evm and long-zero format
…

"before all" hook in "Debug API Test Suite" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests Debug API Test Suite "before all" hook in "Debug API Test Suite"
"before each" hook for "should execute "eth_estimateGas" with both input and data fields present in the txObject" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests "before each" hook for "should execute "eth_estimateGas" with both input and data fields present in the txObject"
"before each" hook for "should execute "eth_getStorageAt" request to get current state changes" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests "before each" hook for "should execute "eth_getStorageAt" request to get current state changes"
"before each" hook: reducing balance for "reverts" ‑ RPC Server Acceptance Tests Acceptance tests @erc20 Acceptance Tests ERC20 Contract should behave like erc20 transfer from when the token owner is not the zero address when the recipient is not the zero address when the spender does not have enough allowance when the token owner does not have enough balance "before each" hook: reducing balance for "reverts"

♻️ This comment has been updated with latest results.

Nana-EC

Good start.
I think we have to think through the way in which this would deploy and scale a bit more.

Nana-EC · 2024-12-19T23:50:56Z

docs/design/additional-endpoints.md

@@ -0,0 +1,160 @@
+# Design Document: ERC20/ERC721/ERC1155 Token Transfer Events API, Tokens Owned by an Address


I would name it Block Explorer API Support

Nana-EC · 2024-12-19T23:51:59Z

docs/design/additional-endpoints.md

+# Design Document: ERC20/ERC721/ERC1155 Token Transfer Events API, Tokens Owned by an Address
+
+## Overview
+- This document outlines the design for implementing new endpoints allowing exposure of more EVM centric data, similar to Etherscan's and BlockScout's APIs.


Suggested change

- This document outlines the design for implementing new endpoints allowing exposure of more EVM centric data, similar to Etherscan's and BlockScout's APIs.

- This document outlines the design for implementing new endpoints allowing exposure of more EVM centric data that support Block Explorer (e.g. Etherscan, BlockScout) API needs.

docs/design/additional-endpoints.md

Nana-EC · 2024-12-19T23:55:24Z

docs/design/additional-endpoints.md

+
+##  `getTokenTransfers`
+
+### Components


This should be a new package not under relay.
We want it to be a separate deployable service so that it can be scaled separately from the relay and not impact the core APIs

If not the alternative is make endpoint support configureable so that a deployemnt can decide to support explorer APIs only or along with core APIs

Reformed the design doc so we implement this as a new package

Nana-EC · 2024-12-19T23:58:02Z

docs/design/additional-endpoints.md

+### Overview
+
+Introduce two new endpoints:
+- `getTokenTransfers` - to fetch token transfer events


Q: what are the params?
You should move the params explanation lower to here.
Also note what's mandatory

So i think in the overview its enough to state the format of the endpoint and the detailed explanation down where it is, do you think thats fine or should I move it all to this section?
Also I have chosen this classic REST format, which differentiates from the way etherscan and blockscout has implemented their endpoints, entirely with query parameters, not sure if thats gonna affect the user. Do they expect the exactly same format
In etherscan the ednpoints look like this:

GET /api ?module=account &action=tokentx &contractaddress=0x9f8f72aa9304c8b593d555f12ef6589cc3a579a2 &address=0x4e83362442b8d1bec281594cea3050c8eb01311c &page=1 &offset=100 &startblock=0 &endblock=27025780 &sort=asc &apikey=YourApiKeyToken

Nana-EC · 2024-12-19T23:59:56Z

docs/design/additional-endpoints.md

+   - Test address matching
+   - Test token standard detection
+
+2. **Acceptance Tests**


Acceptance tests should be in teh form of E2E features a User would go through with a focus on the input and the outputs

Edited them : )

Nana-EC · 2024-12-20T00:00:12Z

docs/design/additional-endpoints.md

+
+### Dependencies
+
+- ERC registry


How so, please expand

+1 on this. Please expand on how ERC Registry is involved.

Nana-EC

Good start waiting on a bit more structure

quiet-node

Great process! Many questions and suggestions

quiet-node · 2025-01-06T21:29:41Z

docs/design/additional-endpoints.md

+
+### Overview
+
+Introduce two new endpoints:


Hmm, this is a bit misleading. The two items below seem more like methods rather than endpoints don’t you think?

quiet-node · 2025-01-06T21:35:06Z

docs/design/additional-endpoints.md

+    MirrorNodeClient->>CommonService: ''
+    CommonService->>EthImpl: ''


Perhaps use a different placeholder instead of an empty string.

quiet-node · 2025-01-06T21:37:52Z

docs/design/additional-endpoints.md

+```mermaid
+sequenceDiagram
+    Client->>EthImpl: getTokenTransfers()
+    EthImpl->>CommonService: getLogs()
+    CommonService->>CommonService: getLogsWithParams()
+    CommonService->>MirrorNodeClient: getContractResultsLogs()
+    MirrorNodeClient->>CommonService: ''
+    CommonService->>EthImpl: ''
+    EthImpl->>EthImpl: getErcRegistry()
+    EthImpl->>EthImpl: checkContracts()
+    EthImpl->>Client: Return Response
+```


Nice diagram, but perhaps the flow from Client to EthImpl should include RelayServer in between:
Client ->> RelayServer: endpoint_values
RelayServer ->> EthImpl: getTokenTransfers()

I also didn’t see anything about getErcRegistry(), checkContracts() until inside the flow diagram, and I don’t believe they are methods implemented in the CommenService or MirrorNodeClient classes. I think it would be better to note down its purpose and clarify.

So in my opinion, since these endpoints are intended to be RESTful, which should not be conflated with the RPC functionality, should we consider creating a new implementation class other than EthImpl dedicated specifically to the REST endpoints? In my view, EthImpl is primarily focused on the RPC functionality, so combining the two within a single implementation class could lead to unnecessary overlap. A separate implementation class for the REST endpoints could leverage shared components like CommonService or MirrorNodeClient while maintaining a clear separation of concerns. What do you think?

Since this will be a standalone package i reformed the design doc and this is not relevant anymore

quiet-node · 2025-01-06T21:46:02Z

docs/design/additional-endpoints.md

+```typescript
+interface TokenTransferEvent {
+  blockNumber: string;
+  timeStamp: string;
+  hash: string;
+  nonce: number;
+  blockHash: string;
+  from: string;
+  contractAddress: string;
+  to: string;
+  tokenId?: string;
+  tokenName?: string;
+  tokenSymbol?: string;
+  tokenDecimal?: number;
+  transactionIndex: number;
+  gas: number;
+  gasPrice: number;
+  gasUsed: number;
+  cumulativeGasUsed: number;
+  input: string;
+  confirmations: number;
+}
+```


Since we’re aiming to match Etherscan’s response interface, should we include a link here to clarify where and how this interface is constructed?

quiet-node · 2025-01-06T21:47:28Z

docs/design/additional-endpoints.md

+
+#### Key Implementation Details
+
+The `getTokenTransfers` endpoint will be calling the same method in the `EthImpl` class which will accept the following parameters:


nit: shouldn't it be method instead of endpoint here?

Suggested change

The `getTokenTransfers` endpoint will be calling the same method in the `EthImpl` class which will accept the following parameters:

The `getTokenTransfers` method will be calling the same method in the `EthImpl` class which will accept the following parameters:

I removed this section, since I reworked the design doc a little bit

docs/design/additional-endpoints.md

quiet-node · 2025-01-06T22:22:41Z

docs/design/additional-endpoints.md

+
+1. Possibly restrict the number of logs returned by the MN to a maximum of 10000
+2. Possibly restrict the block range to a maximum of 10000 blocks
+3. Consider the amount of resources required to check the ERC registry for each log


Could you explain to me what this means?

its not relevant I deleted it

quiet-node · 2025-01-06T22:24:24Z

docs/design/additional-endpoints.md

+
+### Dependencies
+
+- ERC registry


+1 on this. Please expand on how ERC Registry is involved.

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

Nana-EC

Great start, looking forward to discussing more

Nana-EC · 2025-01-08T05:27:26Z

docs/design/additional-endpoints.md

+
+#### 4. CacheService
+Redis-based caching service:
+- Caches frequent queries


Can we call out the cached objects. I'd like to ensure that we only cache what is needed e.g. in some cases the relay caches the full account or token object but we really only need a few fields from the mirror node response.
In this case do we need to cache a full log or can we cache only a subset of them?

No, we dont need to cache the full object, so I changed and specified what we will need

Nana-EC · 2025-01-08T05:27:49Z

docs/design/additional-endpoints.md

+
+#### 4. CacheService
+Redis-based caching service:
+- Caches frequent queries


Please suggest an TTL for these caches components.

Nana-EC · 2025-01-08T05:28:40Z

docs/design/additional-endpoints.md

+
+### Performance Considerations
+
+1. Possibly restrict the number of logs returned by the MN to a maximum of 10000


We currently have a max on the logs endpoint we can likely match that

Currently, investigating this. Seems like its 100 so added this as a value @Nana-EC

Nana-EC · 2025-01-08T05:29:34Z

docs/design/additional-endpoints.md

+ packages/
+   rest-api/
+     └── src/
+         ├── config/           # Configuration for REST API


I don't see a section for this, could you add one that calls out which config fields you plan to add and what their defaults might be?

I added one :) Its a starting point tho, maybe something comes up later during implementation

Nana-EC · 2025-01-08T05:34:59Z

docs/design/additional-endpoints.md

+### Package Structure
+```
+ packages/
+   rest-api/


Hmm, another approach would be rest-server package with the controllers under there and the services under the relay package or this new rest-api package.
In this way the service implementation could be change in the future a transparent way or the underlying server logic - both packages decoupled from each other.
Let's chat about it, the above is food for thought and might hint towards the final form we want to get to in a tentative refactor of the repo

I will add it as a point to discuss in the meeting

Nana-EC · 2025-01-08T05:36:36Z

docs/design/additional-endpoints.md

+
+### Dependencies
+
+* ERC registry - As explained in the _Flow_ section, the TokenService will need to fetch the ERC registry to get the token standard (ERC20/721/ 1155) contracts and filter the response by only those matching the standard, wanted in the request. E.g if the request is for ERC20, the TokenService will need to fetch the ERC registry to get the ERC20 contracts and filter the response from the MN by only those matching the standard.


You should note that initially we'll use the static outputs from the smart contract repo. We should be able to import that package soon but int eh early stage we can copy over the files as needed

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

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

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

sonarqubecloud · 2025-01-10T16:14:03Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

codecov · 2025-01-10T16:55:25Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.66%. Comparing base (13d09e9) to head (e6f883c).
Report is 9 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3342      +/-   ##
==========================================
- Coverage   85.30%   82.66%   -2.64%     
==========================================
  Files          69       65       -4     
  Lines        4688     4541     -147     
  Branches     1050     1036      -14     
==========================================
- Hits         3999     3754     -245     
- Misses        374      421      +47     
- Partials      315      366      +51

Flag	Coverage Δ
config-service	`98.14% <ø> (ø)`
relay	`79.44% <ø> (-0.29%)`	⬇️
server	`?`
ws-server	`?`

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

Files with missing lines	Coverage Δ
packages/relay/src/lib/eth.ts	`81.41% <ø> (-5.27%)`	⬇️

... and 19 files with indirect coverage changes

Nana-EC

Good suggestions @konstantinabl.
To confirm after some thought we should indeed position this from the get go as an independently deployable instance.
Should be something like http-explorer-server containing the specific etherscan focused controllers and server logic.
Initially we can colocate the api logic including services in the relay package then revisit when we pick up the repo refactor

Nana-EC · 2025-01-10T23:47:51Z

docs/design/additional-endpoints.md

+
+| Endpoint | Description |
+|----------|-------------|
+| `GET /api?module=account&action=tokentx&address={address}&contractaddress={contractaddress}&startblock={startblock}&endblock={endblock}&page={page}&offset={offset}&sort={asc\|desc}` | Get ERC20 token transfer events |


I wonder if we should make the etherscan nature explicit
Somethings like what snowtrace does https://testnet.snowtrace.io/documentation/api-swagger

Suggested change

| `GET /api?module=account&action=tokentx&address={address}&contractaddress={contractaddress}&startblock={startblock}&endblock={endblock}&page={page}&offset={offset}&sort={asc\|desc}` | Get ERC20 token transfer events |

| `GET /api/v1/etherscan?module=account&action=tokentx&address={address}&contractaddress={contractaddress}&startblock={startblock}&endblock={endblock}&page={page}&offset={offset}&sort={asc\|desc}` | Get ERC20 token transfer events |

Nana-EC · 2025-01-10T23:52:01Z

docs/design/additional-endpoints.md

+
+Currently MN doesn't support EVM centric queries, like token transfer events for specific standards like ERC20, ERC721, ERC1155 or balance of an address for a specific token.
+
+## Goals


Please take a look at
https://api-testnet.snowtrace.io/api?module=contract&action=getsourcecode&address=0x5425890298aed601595a70AB815c96711a31Bc65, a user may use this when looking for ABI
I think this might be another goal to capture. P2 in nature for this story though.
This is very interesting as it may require calling the verification service.

@acuarica do you see any other option to retrieve such information?

konstantinabl linked an issue Dec 18, 2024 that may be closed by this pull request

Create design doc to support REST API #3340

Open

konstantinabl self-assigned this Dec 18, 2024

konstantinabl added the design Design, pilot and prototyping exploration work label Dec 18, 2024

konstantinabl added this to the 0.63.0 milestone Dec 18, 2024

Adds draft design doc

b840444

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

konstantinabl force-pushed the 3340-create-design-doc-to-support-rest-api branch from 7088d1c to b840444 Compare December 18, 2024 17:40

konstantinabl marked this pull request as ready for review December 18, 2024 22:41

konstantinabl requested review from a team as code owners December 18, 2024 22:41

konstantinabl requested a review from stoyanov-st December 18, 2024 22:41

Nana-EC reviewed Dec 20, 2024

View reviewed changes

Nana-EC requested changes Dec 20, 2024

View reviewed changes

quiet-node modified the milestones: 0.63.0, 0.64.0 Dec 26, 2024

quiet-node reviewed Jan 6, 2025

View reviewed changes

konstantinabl force-pushed the 3340-create-design-doc-to-support-rest-api branch from bc9bfcc to 441d39b Compare January 7, 2025 16:22

Addresses some comments

f81f789

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

konstantinabl force-pushed the 3340-create-design-doc-to-support-rest-api branch from 441d39b to f81f789 Compare January 7, 2025 16:35

Address comments and expand the design doc

800e769

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

konstantinabl requested review from quiet-node, Nana-EC and a team January 7, 2025 21:07

Nana-EC requested a review from acuarica January 8, 2025 05:22

Nana-EC reviewed Jan 8, 2025

View reviewed changes

konstantinabl added 3 commits January 8, 2025 15:54

Addresses comments

f1be973

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

Expands on few sections, fixes endpoint formatting, addresses PR comm…

09f5fa8

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

Adds config variables to design doc

e6f883c

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

konstantinabl requested a review from Nana-EC January 10, 2025 16:14

konstantinabl requested a review from a team January 10, 2025 16:15

Nana-EC reviewed Jan 13, 2025

View reviewed changes

		@@ -0,0 +1,160 @@
		# Design Document: ERC20/ERC721/ERC1155 Token Transfer Events API, Tokens Owned by an Address

	- This document outlines the design for implementing new endpoints allowing exposure of more EVM centric data, similar to Etherscan's and BlockScout's APIs.
	- This document outlines the design for implementing new endpoints allowing exposure of more EVM centric data that support Block Explorer (e.g. Etherscan, BlockScout) API needs.

		MirrorNodeClient->>CommonService: ''
		CommonService->>EthImpl: ''


		#### Key Implementation Details

		The `getTokenTransfers` endpoint will be calling the same method in the `EthImpl` class which will accept the following parameters:

	The `getTokenTransfers` endpoint will be calling the same method in the `EthImpl` class which will accept the following parameters:
	The `getTokenTransfers` method will be calling the same method in the `EthImpl` class which will accept the following parameters:


		### Performance Considerations

		1. Possibly restrict the number of logs returned by the MN to a maximum of 10000


		### Dependencies

		* ERC registry - As explained in the _Flow_ section, the TokenService will need to fetch the ERC registry to get the token standard (ERC20/721/ 1155) contracts and filter the response by only those matching the standard, wanted in the request. E.g if the request is for ERC20, the TokenService will need to fetch the ERC registry to get the ERC20 contracts and filter the response from the MN by only those matching the standard.

	\| `GET /api?module=account&action=tokentx&address={address}&contractaddress={contractaddress}&startblock={startblock}&endblock={endblock}&page={page}&offset={offset}&sort={asc\\|desc}` \| Get ERC20 token transfer events \|
	\| `GET /api/v1/etherscan?module=account&action=tokentx&address={address}&contractaddress={contractaddress}&startblock={startblock}&endblock={endblock}&page={page}&offset={offset}&sort={asc\\|desc}` \| Get ERC20 token transfer events \|


		Currently MN doesn't support EVM centric queries, like token transfer events for specific standards like ERC20, ERC721, ERC1155 or balance of an address for a specific token.

		## Goals

feat: Adds draft design doc for new endpoints #3342

Are you sure you want to change the base?

feat: Adds draft design doc for new endpoints #3342

Conversation

konstantinabl commented Dec 18, 2024 • edited Loading

github-actions bot commented Dec 18, 2024 • edited Loading

Test Results

Nana-EC 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quiet-node Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

Nana-EC left a comment

Choose a reason for hiding this comment

quiet-node left a comment

Choose a reason for hiding this comment

quiet-node Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quiet-node Jan 6, 2025 • edited Loading

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

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

quiet-node Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

Nana-EC 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

konstantinabl Jan 10, 2025 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 10, 2025

Quality Gate passed

codecov bot commented Jan 10, 2025

Codecov Report

Nana-EC left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

konstantinabl commented Dec 18, 2024 •

edited

Loading

github-actions bot commented Dec 18, 2024 •

edited

Loading

quiet-node Jan 6, 2025 •

edited

Loading

quiet-node Jan 6, 2025 •

edited

Loading

quiet-node Jan 6, 2025 •

edited

Loading

quiet-node Jan 6, 2025 •

edited

Loading

konstantinabl Jan 10, 2025 •

edited

Loading