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

Sinner: resolvers and updating db read functions #1514

Merged
merged 8 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions services/sinner/api/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,15 @@ func (t *APISuite) TestGetOrigin() {
err := t.db.StoreOriginSent(t.GetTestContext(), originSent)
Nil(t.T(), err)

result, err := t.sinnerAPI.GetOriginInfo(t.GetTestContext(), txHash, int(chainID))
result, err := t.sinnerAPI.GetOriginInfo(t.GetTestContext(), messageHash)
Nil(t.T(), err)
NotNil(t.T(), result)
Equal(t.T(), txHash, *result.Response.OriginTxHash)

results, err := t.sinnerAPI.GetOriginInfos(t.GetTestContext(), txHash, int(chainID))
Nil(t.T(), err)
NotNil(t.T(), results)
Equal(t.T(), txHash, *results.Response[0].OriginTxHash)
}

func (t *APISuite) TestGetExecuted() {
Expand All @@ -51,10 +56,15 @@ func (t *APISuite) TestGetExecuted() {
err := t.db.StoreExecuted(t.GetTestContext(), executed)
Nil(t.T(), err)

result, err := t.sinnerAPI.GetDestinationInfo(t.GetTestContext(), txHash, int(chainID))
result, err := t.sinnerAPI.GetDestinationInfo(t.GetTestContext(), messageHash)
Nil(t.T(), err)
NotNil(t.T(), result)
Equal(t.T(), txHash, *result.Response.TxHash)

results, err := t.sinnerAPI.GetDestinationInfos(t.GetTestContext(), txHash, int(chainID))
Nil(t.T(), err)
NotNil(t.T(), results)
Equal(t.T(), txHash, *results.Response[0].TxHash)
}

func (t *APISuite) TestMessageStatus() {
Expand Down
5 changes: 5 additions & 0 deletions services/sinner/api/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import (
"context"
"errors"
"fmt"
"github.com/gin-gonic/gin"
"github.com/ipfs/go-log"
Expand All @@ -21,9 +22,13 @@
)

var logger = log.Logger("sinner-api")
var errNoPort = errors.New("port not specified, must be between 1 and 65535")

// Start starts the api server for sinner.
func Start(ctx context.Context, cfg serverConfig.Config, handler metrics.Handler) error {
if cfg.HTTPPort == 0 {
return errNoPort
}

Check warning on line 31 in services/sinner/api/server.go

View check run for this annotation

Codecov / codecov/patch

services/sinner/api/server.go#L30-L31

Added lines #L30 - L31 were not covered by tests
Comment on lines +29 to +31
Copy link
Contributor

Choose a reason for hiding this comment

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

The check for cfg.HTTPPort == 0 is a good addition. It ensures that the server does not start without a valid port. However, it might be beneficial to also check if the port is within the valid range (1-65535). If the port is outside this range, it should also return an error.

if cfg.HTTPPort <= 0 || cfg.HTTPPort > 65535 {
	return errNoPort
}

logger.Warnf("starting api server")
router := ginhelper.New(logger)
// wrap gin with metrics
Expand Down
2 changes: 1 addition & 1 deletion services/sinner/contracts/origin/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (p *ParserImpl) ParseSent(log ethTypes.Log) (*model.OriginSent, error) {
TxHash: iFace.Raw.TxHash.String(),
TxIndex: iFace.Raw.TxIndex,
DestinationChainID: iFace.Destination,
Message: common.Bytes2Hex(iFace.Message[:]),
Message: common.Bytes2Hex(iFace.Message),
Nonce: iFace.Nonce,
MessageHash: common.Bytes2Hex(iFace.MessageHash[:]),
}
Expand Down
14 changes: 8 additions & 6 deletions services/sinner/db/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,21 @@ type EventDBWriter interface {
//
//nolint:interfacebloat
type EventDBReader interface {
// RetrieveOriginSent gets the OriginSent record.
RetrieveOriginSent(ctx context.Context, chainID uint32, txHash string) (model.OriginSent, error)
// RetrieveOriginSent gets the Origin Sent event with a message hash.
RetrieveOriginSent(ctx context.Context, messageHash string) (model.OriginSent, error)
// RetrieveOriginSents gets the Origin Sent events tied to a tx hash.
RetrieveOriginSents(ctx context.Context, chainID uint32, txHash string) ([]model.OriginSent, error)
trajan0x marked this conversation as resolved.
Show resolved Hide resolved
// RetrieveExecuted gets the Executed event with a message hash.
RetrieveExecuted(ctx context.Context, messageHash string) (model.Executed, error)
// RetrieveExecuteds gets the Executed events tied to a tx hash.
RetrieveExecuteds(ctx context.Context, chainID uint32, txHash string) ([]model.Executed, error)
// RetrieveMessageStatus gets status of a message.
RetrieveMessageStatus(ctx context.Context, messageHash string) (graphqlModel.MessageStatus, error)
// RetrieveLastStoredBlock gets the last block stored in sinner.
RetrieveLastStoredBlock(ctx context.Context, chainID uint32, address common.Address) (uint64, error)
// RetrieveExecuted gets the Executed record.
RetrieveExecuted(ctx context.Context, chainID uint32, txHash string) (model.Executed, error)
}

// EventDB stores events.
//
//go:generate go run github.com/vektra/mockery/v2 --name EventDB --output ./mocks --case=underscore
type EventDB interface {
EventDBWriter
EventDBReader
Expand Down
195 changes: 0 additions & 195 deletions services/sinner/db/mocks/event_db.go

This file was deleted.

8 changes: 4 additions & 4 deletions services/sinner/db/model/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ type OriginSent struct {
// BlockNumber is the block number in which the tx occurred.
BlockNumber uint64 `gorm:"column:block_number"`
// TxHash is the hash of the tx.
TxHash string `gorm:"column:tx_hash;primaryKey;index:idx_tx_hash_origin,priority:1,sort:desc"`
TxHash string `gorm:"column:tx_hash;index:idx_tx_hash_origin,priority:1,sort:desc"`
// TxIndex is the index of the tx in a block.
TxIndex uint `gorm:"column:tx_index"`
// Sender is the address of the sender of the tx.
Expand All @@ -76,7 +76,7 @@ type OriginSent struct {
// MessageID is the keccaked message.
MessageID string `gorm:"column:message_id"`
// MessageHash is the message hash.
MessageHash string `gorm:"column:message_hash"`
MessageHash string `gorm:"column:message_hash;primaryKey"`
nautsimon marked this conversation as resolved.
Show resolved Hide resolved
// ChainID is the chain id.
ChainID uint32 `gorm:"column:chain_id;primaryKey;index:idx_tx_hash_origin,priority:2,sort:desc"`
// Destination is the destination chain id.
Expand Down Expand Up @@ -114,11 +114,11 @@ type Executed struct {
// BlockNumber is the block number in which the tx occurred.
BlockNumber uint64 `gorm:"column:block_number"`
// TxHash is the hash of the tx.
TxHash string `gorm:"column:tx_hash;primaryKey;index:idx_tx_hash_executed,priority:1,sort:desc"`
TxHash string `gorm:"column:tx_hash;index:idx_tx_hash_executed,priority:1,sort:desc"`
// TxIndex is the index of the tx in a block.
TxIndex uint `gorm:"column:tx_index"`
// MessageHash is the message hash.
MessageHash string `gorm:"column:message_hash"`
MessageHash string `gorm:"column:message_hash;primaryKey"`
Copy link
Contributor

Choose a reason for hiding this comment

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

should be a foreign key constraint to message table

Copy link
Contributor Author

@nautsimon nautsimon Oct 27, 2023

Choose a reason for hiding this comment

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

Is this for data integrity if deleting from the db? currently the messagestatus table is updated concurrently w/origin/destination table insert while indexing.

An idea could be to do a has many relationship, but the idea of the message status table was to be a lite as possible on query side so it could be pinged a lot for quick updates.

type MessageStatus struct {
	MessageHash        string `gorm:"column:message_hash;uniqueIndex:idx_message_hash_status"`
	OriginTxHash       string `gorm:"column:origin_txhash"`
	DestinationTxHash  string `gorm:"column:destination_txhash"`
	OriginSentMessages []OriginSent `gorm:"foreignKey:MessageHash;references:MessageHash;constraint:OnUpdate:CASCADE,OnDelete:SET NULL;"`
	ExecutedMessages   []Executed   `gorm:"foreignKey:MessageHash;references:MessageHash;constraint:OnUpdate:CASCADE,OnDelete:SET NULL;"`
}

Copy link
Contributor

Choose a reason for hiding this comment

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

has many would be the idea yeah, this will also speed up joins, etc

Copy link
Contributor

Choose a reason for hiding this comment

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

destination tx hash is many to may

// ChainID is the chain id.
ChainID uint32 `gorm:"column:chain_id;primaryKey;index:idx_tx_hash_executed,priority:2,sort:desc"`
// RemoteDomain is the destination.
Expand Down
16 changes: 14 additions & 2 deletions services/sinner/db/read_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,17 @@ func (t *DBSuite) TestRetrieveOriginSent() {
Nil(t.T(), err)

// Test: Retrieve and validate the OriginSent record
retrievedRecord, err := testDB.RetrieveOriginSent(t.GetTestContext(), chainID, txHash)
retrievedRecord, err := testDB.RetrieveOriginSent(t.GetTestContext(), messageHash)
Nil(t.T(), err)
Equal(t.T(), txHash, retrievedRecord.TxHash)
Equal(t.T(), chainID, retrievedRecord.ChainID)
Equal(t.T(), messageHash, retrievedRecord.MessageHash)

retrievedRecords, err := testDB.RetrieveOriginSents(t.GetTestContext(), chainID, txHash)
Nil(t.T(), err)
Equal(t.T(), txHash, retrievedRecords[0].TxHash)
Equal(t.T(), chainID, retrievedRecords[0].ChainID)
Equal(t.T(), messageHash, retrievedRecords[0].MessageHash)
nautsimon marked this conversation as resolved.
Show resolved Hide resolved
})
}

Expand All @@ -118,10 +124,16 @@ func (t *DBSuite) TestRetrieveExecuted() {
Nil(t.T(), err)

// Test: Retrieve and validate the Executed record
retrievedRecord, err := testDB.RetrieveExecuted(t.GetTestContext(), chainID, txHash)
retrievedRecord, err := testDB.RetrieveExecuted(t.GetTestContext(), messageHash)
Nil(t.T(), err)
Equal(t.T(), txHash, retrievedRecord.TxHash)
Equal(t.T(), chainID, retrievedRecord.ChainID)
Equal(t.T(), messageHash, retrievedRecord.MessageHash)

retrievedRecords, err := testDB.RetrieveExecuteds(t.GetTestContext(), chainID, txHash)
Nil(t.T(), err)
Equal(t.T(), txHash, retrievedRecords[0].TxHash)
Equal(t.T(), chainID, retrievedRecords[0].ChainID)
Equal(t.T(), messageHash, retrievedRecords[0].MessageHash)
nautsimon marked this conversation as resolved.
Show resolved Hide resolved
})
}
Loading
Loading