Skip to content

Commit

Permalink
fix review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
temaniarpit27 committed Feb 19, 2025
1 parent e01b00e commit 7da020d
Show file tree
Hide file tree
Showing 12 changed files with 121 additions and 47 deletions.
2 changes: 1 addition & 1 deletion bridgesync/bridgesync.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func (s *BridgeSync) Start(ctx context.Context) {
func (s *BridgeSync) GetBridgesPaged(
ctx context.Context,
page, pageSize uint32,
depositCount uint64,
depositCount *uint64,
) ([]*Bridge, int, error) {
if s.processor.isHalted() {
return nil, 0, sync.ErrInconsistentState
Expand Down
2 changes: 1 addition & 1 deletion bridgesync/bridgesync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,6 @@ func TestGetTokenMappings(t *testing.T) {

func TestGetBridgePaged(t *testing.T) {
s := BridgeSync{processor: &processor{halted: true}}
_, _, err := s.GetBridgesPaged(context.Background(), 0, 0, 0)
_, _, err := s.GetBridgesPaged(context.Background(), 0, 0, nil)
require.ErrorIs(t, err, sync.ErrInconsistentState)
}
8 changes: 8 additions & 0 deletions bridgesync/migrations/bridgesync0002.sql
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
-- +migrate Down
DROP TABLE IF EXISTS token_mapping;
ALTER TABLE bridge DROP COLUMN tx_hash;
ALTER TABLE claim DROP COLUMN tx_hash;
ALTER TABLE bridge DROP COLUMN block_timestamp;
ALTER TABLE claim DROP COLUMN block_timestamp;

-- +migrate Up
CREATE TABLE
Expand All @@ -14,3 +18,7 @@ CREATE TABLE
metadata BLOB,
PRIMARY KEY (block_num, block_pos)
);
ALTER TABLE bridge ADD COLUMN tx_hash VARCHAR;
ALTER TABLE claim ADD COLUMN tx_hash VARCHAR;
ALTER TABLE bridge ADD COLUMN block_timestamp INTEGER;
ALTER TABLE claim ADD COLUMN block_timestamp INTEGER;
11 changes: 0 additions & 11 deletions bridgesync/migrations/bridgesync0003.sql

This file was deleted.

7 changes: 0 additions & 7 deletions bridgesync/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ var mig0001 string
//go:embed bridgesync0002.sql
var mig0002 string

//go:embed bridgesync0003.sql
var mig0003 string

func RunMigrations(dbPath string) error {
migrations := []types.Migration{
{
Expand All @@ -27,10 +24,6 @@ func RunMigrations(dbPath string) error {
ID: "bridgesync0002",
SQL: mig0002,
},
{
ID: "bridgesync0003",
SQL: mig0003,
},
}
migrations = append(migrations, treeMigrations.Migrations...)
return db.RunMigrations(dbPath, migrations)
Expand Down
74 changes: 73 additions & 1 deletion bridgesync/migrations/migrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package migrations

import (
"context"
"math/big"
"path"
"testing"

Expand Down Expand Up @@ -89,6 +90,36 @@ func TestMigration0002(t *testing.T) {
wrapped_token_address,
metadata
) VALUES (1, 0, 1739270804, '0xabcd', 2, '0x3', '0x5', NULL);
INSERT INTO bridge (
block_num,
block_pos,
leaf_type,
origin_network,
origin_address,
destination_network,
destination_address,
amount,
metadata,
deposit_count,
block_timestamp,
tx_hash
) VALUES (1, 0, 0, 0, '0x3', 0, '0x0000', 0, NULL, 0, 1739270804, '0xabcd');
INSERT INTO claim (
block_num,
block_pos,
global_index,
origin_network,
origin_address,
destination_address,
amount,
destination_network,
metadata,
is_message,
block_timestamp,
tx_hash
) VALUES (1, 0, 0, 0, '0x3', '0x0000', 0, 0, NULL, FALSE, 1739270804, '0xabcd');
`)
require.NoError(t, err)
err = tx.Commit()
Expand All @@ -108,12 +139,53 @@ func TestMigration0002(t *testing.T) {
err = meddler.QueryRow(db, &tokenMapping,
`SELECT * FROM token_mapping`)
require.NoError(t, err)

require.NotNil(t, tokenMapping)
require.Equal(t, uint64(1), tokenMapping.BlockNum)
require.Equal(t, uint64(0), tokenMapping.BlockPos)
require.Equal(t, uint64(1739270804), tokenMapping.BlockTimestamp)
require.Equal(t, uint32(2), tokenMapping.OriginNetwork)
require.Equal(t, common.HexToAddress("0x3"), tokenMapping.OriginTokenAddress)
require.Equal(t, common.HexToAddress("0x5"), tokenMapping.WrappedTokenAddress)

var bridge struct {
BlockNum uint64 `meddler:"block_num"`
BlockPos uint64 `meddler:"block_pos"`
LeafType uint8 `meddler:"leaf_type"`
OriginNetwork uint32 `meddler:"origin_network"`
OriginAddress string `meddler:"origin_address"`
DestinationNetwork uint32 `meddler:"destination_network"`
DestinationAddress string `meddler:"destination_address"`
Amount *big.Int `meddler:"amount,bigint"`
Metadata []byte `meddler:"metadata"`
DepositCount uint32 `meddler:"deposit_count"`
BlockTimestamp uint64 `meddler:"block_timestamp"`
TxHash string `meddler:"tx_hash"`
}

err = meddler.QueryRow(db, &bridge,
`SELECT * FROM bridge`)
require.NoError(t, err)
require.NotNil(t, bridge)
require.Equal(t, uint64(1739270804), bridge.BlockTimestamp)

var claim struct {
BlockNum uint64 `meddler:"block_num"`
BlockPos uint64 `meddler:"block_pos"`
GlobalIndex *big.Int `meddler:"global_index,bigint"`
OriginNetwork uint32 `meddler:"origin_network"`
OriginAddress string `meddler:"origin_address"`
DestinationAddress string `meddler:"destination_address"`
Amount *big.Int `meddler:"amount,bigint"`
DestinationNetwork uint32 `meddler:"destination_network"`
Metadata []byte `meddler:"metadata"`
IsMessage bool `meddler:"is_message"`
BlockTimestamp uint64 `meddler:"block_timestamp"`
TxHash string `meddler:"tx_hash"`
}

err = meddler.QueryRow(db, &claim,
`SELECT * FROM claim`)
require.NoError(t, err)
require.NotNil(t, claim)
require.Equal(t, uint64(1739270804), claim.BlockTimestamp)
}
24 changes: 18 additions & 6 deletions bridgesync/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ type Claim struct {
DestinationNetwork uint32 `meddler:"destination_network"`
Metadata []byte `meddler:"metadata"`
IsMessage bool `meddler:"is_message"`
BlockTimestamp uint64 `meddler:"block_timestamp"`
TxHash common.Hash `meddler:"tx_hash,hash"`
}

// TokenMapping representation of a NewWrappedToken event, that is emitted by the bridge contract
Expand Down Expand Up @@ -216,7 +218,7 @@ func (p *processor) GetClaims(
}

func (p *processor) GetBridgesPaged(
ctx context.Context, pageNumber, pageSize uint32, depositCount uint64,
ctx context.Context, pageNumber, pageSize uint32, depositCount *uint64,
) ([]*Bridge, int, error) {
tx, err := p.db.BeginTx(ctx, &sql.TxOptions{ReadOnly: true})
if err != nil {
Expand All @@ -230,25 +232,35 @@ func (p *processor) GetBridgesPaged(
orderBy := "deposit_count"
order := "DESC"
whereClause := ""
count, err := p.GetTotalNumberOfRecords("bridge")
count, err := p.GetTotalNumberOfRecords(bridgeTableName)
if err != nil {
return nil, 0, err
}
if depositCount > 0 {
whereClause = fmt.Sprintf("WHERE deposit_count = %d", depositCount)
if depositCount != nil && *depositCount > 0 {
whereClause = fmt.Sprintf("WHERE deposit_count = %d", *depositCount)
pageNumber = 1
pageSize = 1
}
offset := (pageNumber - 1) * pageSize
rows, err := p.queryPaged(tx, offset, pageSize, "bridge", orderBy, order, whereClause)
if int(offset) >= count {
p.log.Debugf("offset is larger than total bridges (page number=%d, page size=%d, total bridges=%d)",
pageNumber, pageSize, count)
return nil, 0, db.ErrNotFound
}
rows, err := p.queryPaged(tx, offset, pageSize, bridgeTableName, orderBy, order, whereClause)
if err != nil {
return nil, 0, err
}
defer func() {
if err := rows.Close(); err != nil {
p.log.Warnf("error closing rows: %v", err)
}
}()
bridgePtrs := []*Bridge{}
if err = meddler.ScanAll(rows, &bridgePtrs); err != nil {
return nil, 0, err
}
if depositCount > 0 {
if depositCount != nil && *depositCount > 0 {
count = len(bridgePtrs)
}
return bridgePtrs, count, nil
Expand Down
4 changes: 2 additions & 2 deletions bridgesync/processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1037,7 +1037,7 @@ func TestGetBridgesPaged(t *testing.T) {
depositCount: 0,
expectedCount: 6,
expectedBridges: []*Bridge{},
expectedError: nil,
expectedError: db.ErrNotFound,
},
}

Expand All @@ -1048,7 +1048,7 @@ func TestGetBridgesPaged(t *testing.T) {
t.Parallel()

ctx := context.Background()
bridges, count, err := p.GetBridgesPaged(ctx, tc.page, tc.pageSize, tc.depositCount)
bridges, count, err := p.GetBridgesPaged(ctx, tc.page, tc.pageSize, &tc.depositCount)

if tc.expectedError != nil {
require.Equal(t, tc.expectedError, err)
Expand Down
10 changes: 5 additions & 5 deletions rpc/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ type BridgesResult struct {
func (b *BridgeEndpoints) GetBridges(
networkID uint32,
pageNumber, pageSize *uint32,
depositCount uint64,
depositCount *uint64,
) (interface{}, rpc.Error) {
pageNumberU32, pageSizeU32, err := validatePaginationParams(pageNumber, pageSize)
if err != nil {
Expand All @@ -228,9 +228,9 @@ func (b *BridgeEndpoints) GetBridges(
ctx, cancel := context.WithTimeout(context.Background(), b.readTimeout)
defer cancel()

c, merr := b.meter.Int64Counter("getBridges")
c, merr := b.meter.Int64Counter("get_bridges")
if merr != nil {
b.logger.Warnf("failed to create get_token_mappings counter: %s", merr)
b.logger.Warnf("failed to create get_bridges counter: %s", merr)
}
c.Add(ctx, 1)

Expand All @@ -243,12 +243,12 @@ func (b *BridgeEndpoints) GetBridges(
case networkID == 0:
bridges, count, err = b.bridgeL1.GetBridgesPaged(ctx, pageNumberU32, pageSizeU32, depositCount)
if err != nil {
return nil, rpc.NewRPCError(rpc.DefaultErrorCode, fmt.Sprintf("failed to get deposit, error: %s", err))
return nil, rpc.NewRPCError(rpc.DefaultErrorCode, fmt.Sprintf("failed to get bridges, error: %s", err))
}
case networkID == b.networkID:
bridges, count, err = b.bridgeL2.GetBridgesPaged(ctx, pageNumberU32, pageSizeU32, depositCount)
if err != nil {
return nil, rpc.NewRPCError(rpc.DefaultErrorCode, fmt.Sprintf("failed to get deposit, error: %s", err))
return nil, rpc.NewRPCError(rpc.DefaultErrorCode, fmt.Sprintf("failed to get bridges, error: %s", err))
}
default:
return nil, rpc.NewRPCError(
Expand Down
2 changes: 1 addition & 1 deletion rpc/bridge_interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type Bridger interface {
GetBridgesPaged(
ctx context.Context,
pageNumber, pageSize uint32,
depositCount uint64,
depositCount *uint64,
) ([]*bridgesync.Bridge, int, error)
GetTokenMappings(ctx context.Context, pageNumber, pageSize uint32) ([]*bridgesync.TokenMapping, int, error)
}
Expand Down
6 changes: 3 additions & 3 deletions rpc/bridge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ func TestGetBridges(t *testing.T) {
bridgeMocks.bridgeL1.On("GetBridgesPaged", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(bridges, len(bridges), nil)

result, err := bridgeMocks.bridge.GetBridges(0, &page, &pageSize, 0)
result, err := bridgeMocks.bridge.GetBridges(0, &page, &pageSize, nil)
require.NoError(t, err)
require.NotNil(t, result)

Expand Down Expand Up @@ -581,7 +581,7 @@ func TestGetBridges(t *testing.T) {
bridgeMocks.bridgeL2.On("GetBridgesPaged", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(bridges, len(bridges), nil)

result, err := bridgeMocks.bridge.GetBridges(10, &page, &pageSize, 0)
result, err := bridgeMocks.bridge.GetBridges(10, &page, &pageSize, nil)
require.NoError(t, err)
require.NotNil(t, result)

Expand All @@ -596,7 +596,7 @@ func TestGetBridges(t *testing.T) {
t.Run("GetBridges with unsupported network", func(t *testing.T) {
unsupportedNetworkID := uint32(999)

result, err := bridgeMocks.bridge.GetBridges(unsupportedNetworkID, nil, nil, 0)
result, err := bridgeMocks.bridge.GetBridges(unsupportedNetworkID, nil, nil, nil)
require.ErrorContains(t, err, fmt.Sprintf("this client does not support network %d", unsupportedNetworkID))
require.Nil(t, result)
})
Expand Down
18 changes: 9 additions & 9 deletions rpc/mocks/mock_bridger.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 7da020d

Please sign in to comment.