Refactor code #11

0x6e616d · 2024-08-01T17:53:29Z

Add block keeper for syncing the old blocks
Handle re-orged blocks
Refactor code

I tested the deposit and withdraw transaction successfully on my local

Explorer URL: https://sepolia.etherscan.io/tx/0x9ee609a4919f6d9ac3ce673cac0f2f14776acb29c3eabe5fd169f458b9724f20

How to test my PR?

Before you start, please make the docker-compose first to start the redis instance(redis is used for storing the block synced metadata)

docker-compose up -d

After that, make your .env file

## thanos notification env
export NETWORK=sepolia

L1_WS_RPC=wss://ethereum-sepolia-rpc.publicnode.com
L2_WS_RPC=wss://rpc.thanos-sepolia.tokamak.network

L1_STANDARD_BRIDGE=0x385076516318551d566CAaE5EC59c23fe09cbF65
L2_STANDARD_BRIDGE=0x4200000000000000000000000000000000000010

L1_USDC_BRIDGE=0xE390EE020Afb7F8e4A2Dc44a71088db2acd72CF3
L2_USDC_BRIDGE=0x4200000000000000000000000000000000000775

SLACK_URL=<your_slack_hook_url>

L1_EXPLORER_URL=https://sepolia.etherscan.io
L2_EXPLORER_URL=https://explorer.thanos-sepolia.tokamak.network

L1_TOKEN_ADDRESSES=0xa30fe40285B8f5c0457DbC3B7C8A280373c40044,0xFF3Ef745D9878AfE5934Ff0b130868AFDDbc58e8,0x42d3b260c761cD5da022dB56Fe2F89c4A909b04A,0x3D752DE8Ec40B655F21D11b02Fa7438AE18687Fc,0x8c4C0FC89382F96E435527D39C9Ec69dDed34E77,0xf8474C2a90B9035e0b431e1789fE76F54d4ce708,0xfFf9976782d46CC05630D1f6eBAb18b2324d6B14
L2_TOKEN_ADDRESSES=0x4200000000000000000000000000000000000486,0xDeadDeAddeAddEAddeadDEaDDEAdDeaDDeAD0000

REDIS_ADDRESS=localhost:6379

Run the application

go run cmd/app/main.go listener

Enjoy with making some deposit/withdrawal transactions with @tokamak-network/thanos-sdk

nguyenzung · 2024-08-06T08:53:16Z

It is an impressive implementation!

I think there are some things we need to check more. Correct me if I am wrong

There is a gaps time between syncOldBlock and subscribeNewHead, so it is possible that missing some blocks in this period.
In Enqueue(), it is better if not removing oldest blocks. we should remove blocks that need to be removed when reorg occurs. The current impl can lead to a case that a parent hash can not be found in line "if bk.q.Contains(block.Hash().String())"
I remembered that in each toppic, we have a param is called Removed. Maybe we can use this param to detect removed log due to reorg

0x6e616d · 2024-08-06T09:27:22Z

@nguyenzung

there is a gap if the time to fetch the old blocks is too long(over one block time). For example, the old block is 1, but the current block from newHead is 3.
-> It can be lacking with block 2. But we check the reorg blocks every time we receive one block. That means we can consider this case as the re-orged blocks. -> On the handleReorgBlocks function, the parentHash of block 3 is different from the current head(block 1) -> so this code will fetch the block 2 information -> It can be done.

You can test this case by putting time.Sleep on the first line of subscribeNewHead function.

circular_queue is a data structure that will limit the size of the queue. If there is a new one that is inserted into the queue, we will remove the first one to keep the size isn't over.
(https://en.wikipedia.org/wiki/Circular_buffer)

In our case, the block queue used on the block keeper is the circular queue that stores the 64 latest blocks. We assume that after 64 blocks, the chain is immutable
-> In the case of re-orged, we find the re-orged blocks, after that, we will loop the reorged blocks and override the new head.
For example:

1. Current circular queue:
1-2-3-4-5

(The correct chain is 1-2-3-4'-5')
2. We receive block 6' -> the parent hash isn't block hash of 5 -> reorg happened
3. We loop to find the canonical chain -> fetch block 5' with hash is the parentHash of 6
4. Until the parentHash of 4' is the hash of 3. We finish.
5. Order them
6. Loop 4' -> 5' -> 6', each block we set head to the circular queue to 1->2-3-4'-5'-6' 
7. We receive block 7' with the parentHash of 7' as the hash of 6' -> continue this process.

-> So we finish the process of finding the re-orged blocks, we will have the correct queue.

Yes, I missed it, let me update it. We can use log.Remove as true or false to know if this event has been removed or added.

internal/pkg/listener/service.go

0x6e616d · 2024-08-06T09:48:37Z

@nguyenzung Oh I got your point 2. Let me think about it. You're correct. We should remove the reorged block on the queue not the first element. Good point

go.work.sum

…locks

0x6e616d

@nguyenzung many thanks @nguyenzung for your helpful comments

The gap between the time we run syncOldBlocks function and subscribeNewHead can be fixed by the function handleReorgedBlocks by checking whether the parentHash exists on the queue or not.
I updated the queue by adding the new remove function to remove the element by the value. The purpose of this function is to remove the re-orged block hashes if exist not the first element on the queue.

Also, I initiated the block-keeper to get the latest 64 blocks(2 epochs) when starting the event listener service. The purpose of it is to check the re-org of blocks when starting the event listener from scratch or continue from the starting point.

I updated the code to ignore the Removed log when true

0x6e616d · 2024-08-07T07:20:14Z

internal/pkg/listener/service.go

+		if l.Removed {
+			continue
+		}


0x6e616d · 2024-08-07T07:20:34Z

internal/pkg/queue/circular_queue.go

+func (q *CircularQueue[T]) RemoveAndEnqueue(value T, removed T) {
+	q.Remove(removed)
+	q.Enqueue(value)
+}


0x6e616d · 2024-08-07T07:21:13Z

internal/pkg/repository/block_keeper.go

+	for i := uint64(blockNo) - TwoEpochBlocks + 1; i < blockNo; i = i + batchBlocksSize {
+		from := i
+		to := i + batchBlocksSize - 1
+		if to > blockNo-1 {
+			to = blockNo - 1
+		}
+
+		blocks, err := bcSource.GetBlocks(ctx, false, from, to)
+		if err != nil {
+			return nil, err
+		}
+
+		for _, block := range blocks {
+			keeper.enqueue(block.Header.Hash(), block.Header.Number.Uint64())
+		}
+	}
+
+	keeper.enqueue(common.HexToHash(currentBlockHash), blockNo)
+


nguyenzung · 2024-08-07T11:00:05Z

It looks great, Nam!

I think maybe we should have other ticket for UnitTest to make sure everything works well

0x6e616d added 2 commits July 31, 2024 12:07

feat: add erc20 client to fetch the token info

23064c7

feat: return error when starting new service failed

f815131

0x6e616d changed the title ~~Refactor code~~ WIP: Refactor code Aug 1, 2024

0x6e616d added 2 commits August 6, 2024 10:11

feat: add block keeper and sync old blocks if missing

4ec0aca

feat: seperate deposit/withdraw functions

3da016e

0x6e616d changed the title ~~WIP: Refactor code~~ Refactor code Aug 6, 2024

0x6e616d requested a review from a team August 6, 2024 07:41

0x6e616d self-assigned this Aug 6, 2024

0x6e616d added 2 commits August 6, 2024 14:50

add docker-compose file

1d3ea7f

chore: remove unused constants

89f391a

0x6e616d closed this Aug 6, 2024

0x6e616d reopened this Aug 6, 2024

0x6e616d commented Aug 6, 2024

View reviewed changes

internal/pkg/listener/service.go Outdated Show resolved Hide resolved

This comment was marked as duplicate.

Sign in to view

nguyenzung requested review from ohbyeongmin, xxeonge and theo-learner August 6, 2024 10:14

steven94kr reviewed Aug 7, 2024

View reviewed changes

go.work.sum Outdated Show resolved Hide resolved

0x6e616d added 3 commits August 7, 2024 14:05

feat: remove reorged hash in the queue, init the queue with 2 epoch b…

2622ced

…locks

chore: ignore removed logs

9539503

chore: remove go.work.sum

bd527db

0x6e616d commented Aug 7, 2024

View reviewed changes

nguyenzung self-requested a review August 7, 2024 11:00

nguyenzung approved these changes Aug 7, 2024

View reviewed changes

0x6e616d merged commit e87ec07 into main Aug 8, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor code #11

Refactor code #11

0x6e616d commented Aug 1, 2024 •

edited by ohbyeongmin

Loading

nguyenzung commented Aug 6, 2024

0x6e616d commented Aug 6, 2024 •

edited

Loading

This comment was marked as duplicate.

0x6e616d commented Aug 6, 2024

0x6e616d left a comment •

edited

Loading

0x6e616d Aug 7, 2024

0x6e616d Aug 7, 2024

0x6e616d Aug 7, 2024

nguyenzung commented Aug 7, 2024

Refactor code #11

Refactor code #11

Conversation

0x6e616d commented Aug 1, 2024 • edited by ohbyeongmin Loading

How to test my PR?

nguyenzung commented Aug 6, 2024

0x6e616d commented Aug 6, 2024 • edited Loading

This comment was marked as duplicate.

0x6e616d commented Aug 6, 2024

0x6e616d left a comment • edited Loading

Choose a reason for hiding this comment

0x6e616d Aug 7, 2024

Choose a reason for hiding this comment

0x6e616d Aug 7, 2024

Choose a reason for hiding this comment

0x6e616d Aug 7, 2024

Choose a reason for hiding this comment

nguyenzung commented Aug 7, 2024

0x6e616d commented Aug 1, 2024 •

edited by ohbyeongmin

Loading

0x6e616d commented Aug 6, 2024 •

edited

Loading

0x6e616d left a comment •

edited

Loading