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

Add RFQ Guard #2840

Merged
merged 40 commits into from
Jul 6, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
b69063b
WIP: guard skeleton
dwasse Jul 2, 2024
1c4ef5f
WIP: add guarddb package
dwasse Jul 2, 2024
43d10b7
WIP: db loop
dwasse Jul 2, 2024
49e9466
Feat: add BridgeRequest model
dwasse Jul 2, 2024
85cbf82
Feat: store bridge request
dwasse Jul 2, 2024
35bf205
Feat: add dispute trigger
dwasse Jul 2, 2024
ebb8847
Feat: handle disputed log
dwasse Jul 2, 2024
1aea042
Feat: implement relayMatchesBridgeRequest()
dwasse Jul 2, 2024
9c54eee
Fix: build
dwasse Jul 3, 2024
bdbaa7e
Fix: guarddb models
dwasse Jul 3, 2024
7864840
Feat: check verified status in e2e test
dwasse Jul 3, 2024
ff7d1ca
Feat: check verified status in TestETHtoETH
dwasse Jul 3, 2024
b2d25e9
Feat: add guard cmd
dwasse Jul 3, 2024
76b2e1a
Feat: add embedded guard to relayer
dwasse Jul 3, 2024
278cbb9
Feat: add guard config
dwasse Jul 3, 2024
568acce
Feat: add converter for relayer cfg -> guard cfg
dwasse Jul 3, 2024
24c551e
Feat: add UseEmbeddedGuard flag
dwasse Jul 3, 2024
99834de
Feat: add guard wallet for e2e
dwasse Jul 3, 2024
a9174c3
WIP: add TestDispute
dwasse Jul 3, 2024
11196b4
Feat: start relayer / guard within tests
dwasse Jul 3, 2024
880e044
Fix: return valid=false on 'not found' err
dwasse Jul 3, 2024
32ede77
Fix: pass in raw request
dwasse Jul 3, 2024
6ab95e1
Cleanup: logs
dwasse Jul 3, 2024
c08e4f5
Feat: add tracing
dwasse Jul 3, 2024
b344694
Cleanup: move handlers to handlers.go
dwasse Jul 3, 2024
cdd769d
Cleanup: lint
dwasse Jul 4, 2024
bb677e0
Cleanup: lint
dwasse Jul 5, 2024
ed153b7
[goreleaser]
dwasse Jul 5, 2024
3ab1ce6
Merge branch 'master' into feat/rfq-guard
dwasse Jul 5, 2024
afb427b
[goreleaser]
dwasse Jul 5, 2024
6efd100
Fix: inherit txSubmitter from relayer
dwasse Jul 5, 2024
f5a00e6
[goreleaser]
dwasse Jul 5, 2024
d3011d8
add guard
trajan0x Jul 5, 2024
6982d44
use correct transactor
trajan0x Jul 5, 2024
3544a11
merge master [goreleaser]
trajan0x Jul 6, 2024
9b22148
event parsing
trajan0x Jul 6, 2024
e576f34
Merge branch 'master' into feat/rfq-guard [goreleaser]
trajan0x Jul 6, 2024
7ceb0db
fix tests
trajan0x Jul 6, 2024
71133e6
anvil removal [goreleaser]
trajan0x Jul 6, 2024
63e553c
ci again
trajan0x Jul 6, 2024
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
44 changes: 44 additions & 0 deletions services/rfq/guard/guarddb/base/bridge.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package base

import (
"context"
"errors"
"fmt"

"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/synapsecns/sanguine/services/rfq/guard/guarddb"
"gorm.io/gorm"
"gorm.io/gorm/clause"
)

// StoreBridgeRequest stores a quote request.
func (s Store) StoreBridgeRequest(ctx context.Context, request guarddb.BridgeRequest) error {
model := FromBridgeRequest(request)
dbTx := s.DB().WithContext(ctx).Clauses(clause.OnConflict{
Columns: []clause.Column{{Name: transactionIDFieldName}},
DoUpdates: clause.AssignmentColumns([]string{transactionIDFieldName}),
}).Create(&model)
if dbTx.Error != nil {
Copy link

Choose a reason for hiding this comment

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

🪶 style: 🪶 style: Include the original error in the returned error message for better debugging.

return fmt.Errorf("could not store request: %w", dbTx.Error)
}
return nil
}

// GetBridgeRequestByID gets a quote request by id. Should return ErrNoBridgeRequestForID if not found.
func (s Store) GetBridgeRequestByID(ctx context.Context, id [32]byte) (*guarddb.BridgeRequest, error) {
var modelResult BridgeRequestModel
tx := s.DB().WithContext(ctx).Where(fmt.Sprintf("%s = ?", transactionIDFieldName), hexutil.Encode(id[:])).First(&modelResult)
if errors.Is(tx.Error, gorm.ErrRecordNotFound) {
return nil, guarddb.ErrNoBridgeRequestForID
}

if tx.Error != nil {
return nil, fmt.Errorf("could not get request")
Copy link

Choose a reason for hiding this comment

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

🪶 style: Include the original error in the returned error message for better debugging.

Copy link

Choose a reason for hiding this comment

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

🪶 style: 🪶 style: Wrap the error with additional context for better error tracing.

}

qr, err := modelResult.ToBridgeRequest()
if err != nil {
return nil, err
Copy link

Choose a reason for hiding this comment

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

🪶 style: Wrap the error with additional context for better error tracing.

}
return qr, nil
}
2 changes: 2 additions & 0 deletions services/rfq/guard/guarddb/base/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Package base contains the base implementation for different sql driers.
Copy link

Choose a reason for hiding this comment

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

📚 spelling: Typo in 'driers'. It should be 'drivers'.

package base
215 changes: 215 additions & 0 deletions services/rfq/guard/guarddb/base/model.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
package base

import (
"database/sql"
"errors"
"fmt"
"math/big"
"time"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/synapsecns/sanguine/core/dbcommon"
"github.com/synapsecns/sanguine/services/rfq/contracts/fastbridge"
"github.com/synapsecns/sanguine/services/rfq/guard/guarddb"
)

func init() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add test coverage for the init function.

The init function initializes consistent names for fields, but it lacks test coverage.

// Add tests to ensure the consistent naming initialization works as expected.

Committable suggestion was skipped due to low confidence.

Tools
GitHub Check: codecov/patch

[warning] 16-19: services/rfq/guard/guarddb/base/model.go#L16-L19
Added lines #L16 - L19 were not covered by tests

namer := dbcommon.NewNamer(GetAllModels())
statusFieldName = namer.GetConsistentName("Status")
transactionIDFieldName = namer.GetConsistentName("TransactionID")
originTxHashFieldName = namer.GetConsistentName("OriginTxHash")
destTxHashFieldName = namer.GetConsistentName("DestTxHash")
rebalanceIDFieldName = namer.GetConsistentName("RebalanceID")
relayNonceFieldName = namer.GetConsistentName("RelayNonce")
}

var (
// statusFieldName is the status field name.
statusFieldName string
// transactionIDFieldName is the transactions id field name.
transactionIDFieldName string
// originTxHashFieldName is the origin tx hash field name.
originTxHashFieldName string
// destTxHashFieldName is the dest tx hash field name.
destTxHashFieldName string
// rebalanceIDFieldName is the rebalances id field name.
rebalanceIDFieldName string
// relayNonceFieldName is the relay nonce field name.
relayNonceFieldName string
)

// PendingProvenModel is the primary event model.
type PendingProvenModel struct {
// CreatedAt is the creation time
CreatedAt time.Time
// UpdatedAt is the update time
UpdatedAt time.Time
// Origin is the origin chain id
Origin uint32
// TransactionID is the transaction id of the event
TransactionID string `gorm:"column:transaction_id;primaryKey"`
// TxHash is the hash of the relay transaction on destination
TxHash string
// Status is the status of the event
Status guarddb.PendingProvenStatus
}

// FromPendingProven converts a quote request to an object that can be stored in the db.
func FromPendingProven(proven guarddb.PendingProven) PendingProvenModel {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add test coverage for the FromPendingProven function.

The FromPendingProven function converts a PendingProven object to a PendingProvenModel object, but it lacks test coverage.

// Add tests to ensure the conversion from PendingProven to PendingProvenModel works as expected.

Committable suggestion was skipped due to low confidence.

Tools
GitHub Check: codecov/patch

[warning] 46-52: services/rfq/guard/guarddb/base/model.go#L46-L52
Added lines #L46 - L52 were not covered by tests

return PendingProvenModel{
Origin: proven.Origin,
TransactionID: hexutil.Encode(proven.TransactionID[:]),
TxHash: proven.TxHash.Hex(),
Status: proven.Status,
}
}

var emptyHash = common.HexToHash("").Hex()

func hashToNullString(h common.Hash) sql.NullString {
if h.Hex() == emptyHash {
return sql.NullString{Valid: false}
}
return sql.NullString{
String: h.Hex(),
Valid: true,
}
}

func stringToNullString(s string) sql.NullString {
if s == "" {
return sql.NullString{Valid: false}
}
return sql.NullString{
String: s,
Valid: true,
}
}

// ToPendingProven converts a db object to a pending proven.
func (p PendingProvenModel) ToPendingProven() (*guarddb.PendingProven, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add test coverage for the ToPendingProven function.

The ToPendingProven function converts a PendingProvenModel object to a PendingProven object, but it lacks test coverage.

// Add tests to ensure the conversion from PendingProvenModel to PendingProven works as expected.

Improve error messages for better context.

Include more context in the error messages to make debugging easier.

-	if err != nil {
-		return nil, fmt.Errorf("could not get transaction id: %w", err)
+	if err != nil {
+		return nil, fmt.Errorf("could not get transaction id (%s): %w", p.TransactionID, err)
	}
-	if err != nil {
-		return nil, fmt.Errorf("could not convert transaction id: %w", err)
+	if err != nil {
+		return nil, fmt.Errorf("could not convert transaction id (%s): %w", p.TransactionID, err)
	}
Tools
GitHub Check: codecov/patch

[warning] 56-60: services/rfq/guard/guarddb/base/model.go#L56-L60
Added lines #L56 - L60 were not covered by tests

txID, err := hexutil.Decode(p.TransactionID)
if err != nil {
return nil, fmt.Errorf("could not get transaction id: %w", err)
Comment on lines +57 to +59
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Error handling for hexutil.Decode should be more descriptive.

Comment on lines +57 to +59
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Error handling for hexutil.Decode should be more descriptive.

Comment on lines +57 to +59
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error message for transaction ID decoding.

Include the transaction ID in the error message for better context.

-	if err != nil {
-		return nil, fmt.Errorf("could not get transaction id: %w", err)
-	}
+	if err != nil {
+		return nil, fmt.Errorf("could not get transaction id (%s): %w", p.TransactionID, err)
+	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
txID, err := hexutil.Decode(p.TransactionID)
if err != nil {
return nil, fmt.Errorf("could not get transaction id: %w", err)
txID, err := hexutil.Decode(p.TransactionID)
if err != nil {
return nil, fmt.Errorf("could not get transaction id (%s): %w", p.TransactionID, err)

Comment on lines +57 to +59
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Error handling for hexutil.Decode should be more descriptive.

}

transactionID, err := sliceToArray(txID)
if err != nil {
return nil, fmt.Errorf("could not convert transaction id: %w", err)
Comment on lines +62 to +64
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Consider adding more context to the error message for sliceToArray.

Comment on lines +62 to +64
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Consider adding more context to the error message for sliceToArray.

Comment on lines +62 to +64
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error message for transaction ID conversion.

Include the transaction ID in the error message for better context.

-	if err != nil {
-		return nil, fmt.Errorf("could not convert transaction id: %w", err)
-	}
+	if err != nil {
+		return nil, fmt.Errorf("could not convert transaction id (%s): %w", p.TransactionID, err)
+	}

Committable suggestion was skipped due to low confidence.

Comment on lines +62 to +64
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Consider adding more context to the error message for sliceToArray.

}

return &guarddb.PendingProven{
Origin: p.Origin,
TransactionID: transactionID,
TxHash: common.HexToHash(p.TxHash),
Status: p.Status,
}, nil
}

// BridgeRequestModel is the primary event model.
type BridgeRequestModel struct {
// CreatedAt is the creation time
CreatedAt time.Time
// UpdatedAt is the update time
UpdatedAt time.Time
// TransactionID is the transaction id of the event
TransactionID string `gorm:"column:transaction_id;primaryKey"`
// OriginChainID is the origin chain for the transactions
OriginChainID uint32
// DestChainID is the destination chain for the tx
DestChainID uint32
// OriginSender is the original sender
OriginSender string
// DestRecipient is the recipient of the destination tx
DestRecipient string
// OriginToken is the origin token address
OriginToken string
// DestToken is the destination token address
DestToken string
// OriginAmount is the origin amount stored for sorting.
// This is not the source of truth, but is approximate
OriginAmount string
// DestAmount is the destination amount stored for sorting.
DestAmount string
// Deadline is the deadline for the relay
Deadline time.Time `gorm:"index"`
// OriginNonce is the nonce on the origin chain in the app.
// this is not effected by the message.sender nonce.
OriginNonce int `gorm:"index"`
// RawRequest is the raw request, hex encoded.
RawRequest string
// SendChainGas is true if the chain should send gas
SendChainGas bool
}

func FromBridgeRequest(request guarddb.BridgeRequest) BridgeRequestModel {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add test coverage for the FromBridgeRequest function.

The FromBridgeRequest function converts a BridgeRequest object to a BridgeRequestModel object, but it lacks test coverage.

// Add tests to ensure the conversion from BridgeRequest to BridgeRequestModel works as expected.

Add a comment for the function.

The function FromBridgeRequest is missing a comment.

// FromBridgeRequest converts a BridgeRequest object to a BridgeRequestModel object.
Tools
GitHub Check: codecov/patch

[warning] 112-127: services/rfq/guard/guarddb/base/model.go#L112-L127
Added lines #L112 - L127 were not covered by tests

return BridgeRequestModel{
TransactionID: hexutil.Encode(request.TransactionID[:]),
OriginChainID: request.Transaction.OriginChainId,
DestChainID: request.Transaction.DestChainId,
OriginSender: request.Transaction.OriginSender.String(),
DestRecipient: request.Transaction.DestRecipient.String(),
OriginToken: request.Transaction.OriginToken.String(),
RawRequest: hexutil.Encode(request.RawRequest),
SendChainGas: request.Transaction.SendChainGas,
DestToken: request.Transaction.DestToken.String(),
OriginAmount: request.Transaction.OriginAmount.String(),
DestAmount: request.Transaction.DestAmount.String(),
Deadline: time.Unix(int64(request.Transaction.Deadline.Uint64()), 0),
OriginNonce: int(request.Transaction.Nonce.Uint64()),
}
}

func (b BridgeRequestModel) ToBridgeRequest() (*guarddb.BridgeRequest, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add test coverage for the ToBridgeRequest function.

The ToBridgeRequest function converts a BridgeRequestModel object to a BridgeRequest object, but it lacks test coverage.

// Add tests to ensure the conversion from BridgeRequestModel to BridgeRequest works as expected.

Add a comment for the function.

The function ToBridgeRequest is missing a comment.

// ToBridgeRequest converts a BridgeRequestModel object to a BridgeRequest object.

Improve error messages for better context.

Include more context in the error messages to make debugging easier.

-	if err != nil {
-		return nil, fmt.Errorf("could not get transaction id: %w", err)
+	if err != nil {
+		return nil, fmt.Errorf("could not get transaction id (%s): %w", b.TransactionID, err)
	}
-	if err != nil {
-		return nil, fmt.Errorf("could not get request: %w", err)
+	if err != nil {
+		return nil, fmt.Errorf("could not get request (%s): %w", b.RawRequest, err)
	}
-	if err != nil {
-		return nil, fmt.Errorf("could not convert transaction id: %w", err)
+	if err != nil {
+		return nil, fmt.Errorf("could not convert transaction id (%s): %w", b.TransactionID, err)
	}
-	if !ok {
-		return nil, errors.New("could not convert origin amount")
+	if !ok {
+		return nil, fmt.Errorf("could not convert origin amount (%s)", b.OriginAmount)
	}
-	if !ok {
-		return nil, errors.New("could not convert dest amount")
+	if !ok {
+		return nil, fmt.Errorf("could not convert dest amount (%s)", b.DestAmount)
	}
Tools
GitHub Check: codecov/patch

[warning] 131-135: services/rfq/guard/guarddb/base/model.go#L131-L135
Added lines #L131 - L135 were not covered by tests

txID, err := hexutil.Decode(b.TransactionID)
if err != nil {
return nil, fmt.Errorf("could not get transaction id: %w", err)
}
Comment on lines +133 to +135
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Error handling for hexutil.Decode should be more descriptive.

Comment on lines +133 to +135
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Error handling for hexutil.Decode should be more descriptive.

Comment on lines +133 to +135
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error message for transaction ID decoding.

Include the transaction ID in the error message for better context.

-	if err != nil {
-		return nil, fmt.Errorf("could not get transaction id: %w", err)
-	}
+	if err != nil {
+		return nil, fmt.Errorf("could not get transaction id (%s): %w", b.TransactionID, err)
+	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if err != nil {
return nil, fmt.Errorf("could not get transaction id: %w", err)
}
if err != nil {
return nil, fmt.Errorf("could not get transaction id (%s): %w", b.TransactionID, err)
}

Comment on lines +133 to +135
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Error handling for hexutil.Decode should be more descriptive.


req, err := hexutil.Decode(b.RawRequest)
if err != nil {
return nil, fmt.Errorf("could not get request: %w", err)
}
Comment on lines +138 to +140
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error message for request decoding.

Include the raw request in the error message for better context.

-	if err != nil {
-		return nil, fmt.Errorf("could not get request: %w", err)
-	}
+	if err != nil {
+		return nil, fmt.Errorf("could not get request (%s): %w", b.RawRequest, err)
+	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if err != nil {
return nil, fmt.Errorf("could not get request: %w", err)
}
if err != nil {
return nil, fmt.Errorf("could not get request (%s): %w", b.RawRequest, err)
}


transactionID, err := sliceToArray(txID)
if err != nil {
return nil, fmt.Errorf("could not convert transaction id: %w", err)
Comment on lines +142 to +144
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Consider adding more context to the error message for sliceToArray.

Comment on lines +142 to +144
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Consider adding more context to the error message for sliceToArray.

Comment on lines +142 to +144
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error message for transaction ID conversion.

Include the transaction ID in the error message for better context.

-	if err != nil {
-		return nil, fmt.Errorf("could not convert transaction id: %w", err)
-	}
+	if err != nil {
+		return nil, fmt.Errorf("could not convert transaction id (%s): %w", b.TransactionID, err)
+	}

Committable suggestion was skipped due to low confidence.

Comment on lines +142 to +144
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Consider adding more context to the error message for sliceToArray.

}

originAmount, ok := new(big.Int).SetString(b.OriginAmount, 10)
if !ok {
return nil, errors.New("could not convert origin amount")
}
Comment on lines +148 to +150
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Consider adding more context to the error message for SetString conversion.

Comment on lines +148 to +150
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Consider adding more context to the error message for SetString conversion.

Comment on lines +148 to +150
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error message for origin amount conversion.

Include the origin amount in the error message for better context.

-	if !ok {
-		return nil, errors.New("could not convert origin amount")
-	}
+	if !ok {
+		return nil, fmt.Errorf("could not convert origin amount (%s)", b.OriginAmount)
+	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if !ok {
return nil, errors.New("could not convert origin amount")
}
if !ok {
return nil, fmt.Errorf("could not convert origin amount (%s)", b.OriginAmount)
}

Comment on lines +148 to +150
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Consider adding more context to the error message for SetString conversion.

destAmount, ok := new(big.Int).SetString(b.DestAmount, 10)
if !ok {
return nil, errors.New("could not convert dest amount")
}
Comment on lines +152 to +154
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Consider adding more context to the error message for SetString conversion.

Comment on lines +152 to +154
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Consider adding more context to the error message for SetString conversion.

Comment on lines +152 to +154
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error message for destination amount conversion.

Include the destination amount in the error message for better context.

-	if !ok {
-		return nil, errors.New("could not convert dest amount")
-	}
+	if !ok {
+		return nil, fmt.Errorf("could not convert dest amount (%s)", b.DestAmount)
+	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if !ok {
return nil, errors.New("could not convert dest amount")
}
if !ok {
return nil, fmt.Errorf("could not convert dest amount (%s)", b.DestAmount)
}

Comment on lines +152 to +154
Copy link

Choose a reason for hiding this comment

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

🧠 logic: Consider adding more context to the error message for SetString conversion.


return &guarddb.BridgeRequest{
TransactionID: transactionID,
RawRequest: req,
Transaction: fastbridge.IFastBridgeBridgeTransaction{
OriginChainId: b.OriginChainID,
DestChainId: b.DestChainID,
OriginSender: common.HexToAddress(b.OriginSender),
DestRecipient: common.HexToAddress(b.DestRecipient),
OriginToken: common.HexToAddress(b.OriginToken),
SendChainGas: b.SendChainGas,
DestToken: common.HexToAddress(b.DestToken),
OriginAmount: originAmount,
DestAmount: destAmount,
Deadline: big.NewInt(b.Deadline.Unix()),
Nonce: big.NewInt(int64(b.OriginNonce)),
},
}, nil
}

func sliceToArray(slice []byte) ([32]byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add test coverage for the sliceToArray function.

The sliceToArray function converts a byte slice to a fixed-size array, but it lacks test coverage.

// Add tests to ensure the sliceToArray function works as expected.

Improve error message for slice length validation.

Include the slice length in the error message for better context.

-	if len(slice) != 32 {
-		return arr, errors.New("slice is not 32 bytes long")
+	if len(slice) != 32 {
+		return arr, fmt.Errorf("slice is not 32 bytes long (length: %d)", len(slice))
	}
Tools
GitHub Check: codecov/patch

[warning] 175-181: services/rfq/guard/guarddb/base/model.go#L175-L181
Added lines #L175 - L181 were not covered by tests

var arr [32]byte
if len(slice) != 32 {
return arr, errors.New("slice is not 32 bytes long")
Comment on lines +177 to +178
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error message for slice length validation.

Include the slice length in the error message for better context.

-	if len(slice) != 32 {
-		return arr, errors.New("slice is not 32 bytes long")
-	}
+	if len(slice) != 32 {
+		return arr, fmt.Errorf("slice is not 32 bytes long (length: %d)", len(slice))
+	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if len(slice) != 32 {
return arr, errors.New("slice is not 32 bytes long")
if len(slice) != 32 {
return arr, fmt.Errorf("slice is not 32 bytes long (length: %d)", len(slice))
}

}
copy(arr[:], slice)
return arr, nil
}
80 changes: 80 additions & 0 deletions services/rfq/guard/guarddb/base/proven.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package base

import (
"context"
"errors"
"fmt"

"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/synapsecns/sanguine/services/rfq/guard/guarddb"
"gorm.io/gorm"
"gorm.io/gorm/clause"
)

// StorePendingProven stores a quote request.
func (s Store) StorePendingProven(ctx context.Context, proven guarddb.PendingProven) error {
model := FromPendingProven(proven)
dbTx := s.DB().WithContext(ctx).Clauses(clause.OnConflict{
Columns: []clause.Column{{Name: transactionIDFieldName}},
DoUpdates: clause.AssignmentColumns([]string{transactionIDFieldName}),
}).Create(&model)
if dbTx.Error != nil {
return fmt.Errorf("could not store proven: %w", dbTx.Error)
}
return nil
}

// UpdatePendingProvenStatus updates the status of a pending proven.
func (s Store) UpdatePendingProvenStatus(ctx context.Context, id [32]byte, status guarddb.PendingProvenStatus) error {
tx := s.DB().WithContext(ctx).Model(&PendingProvenModel{}).
Where(fmt.Sprintf("%s = ?", transactionIDFieldName), hexutil.Encode(id[:])).
Update(statusFieldName, status)
if tx.Error != nil {
return fmt.Errorf("could not update: %w", tx.Error)
}
return nil
}
Comment on lines +28 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

Use parameterized query for WHERE clause.

The WHERE clause should use parameterized queries to prevent SQL injection.

- .Where(fmt.Sprintf("%s = ?", transactionIDFieldName), hexutil.Encode(id[:])).
+ .Where(fmt.Sprintf("%s = ?", transactionIDFieldName), hexutil.Encode(id[:])).

Committable suggestion was skipped due to low confidence.


// GetPendingProvensByStatus gets pending provens by status.
func (s Store) GetPendingProvensByStatus(ctx context.Context, matchStatuses ...guarddb.PendingProvenStatus) (res []guarddb.PendingProven, _ error) {
var provenResults []PendingProvenModel

inArgs := make([]int, len(matchStatuses))
for i := range matchStatuses {
inArgs[i] = int(matchStatuses[i].Int())
}

// TODO: consider pagination
tx := s.DB().WithContext(ctx).Model(&PendingProvenModel{}).Where(fmt.Sprintf("%s IN ?", statusFieldName), inArgs).Find(&provenResults)
if tx.Error != nil {
return []guarddb.PendingProven{}, fmt.Errorf("could not get db results: %w", tx.Error)
}

for _, result := range provenResults {
marshaled, err := result.ToPendingProven()
if err != nil {
return []guarddb.PendingProven{}, fmt.Errorf("could not get provens")
}
res = append(res, *marshaled)
}
return res, nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider implementing pagination.

The TODO comment suggests considering pagination for large result sets. Implementing pagination would improve performance and prevent potential memory issues.

Do you want me to help implement pagination for this method?


// GetPendingProvenByID gets a quote request by id. Should return ErrNoProvenForID if not found.
func (s Store) GetPendingProvenByID(ctx context.Context, id [32]byte) (*guarddb.PendingProven, error) {
var modelResult PendingProvenModel
tx := s.DB().WithContext(ctx).Where(fmt.Sprintf("%s = ?", transactionIDFieldName), hexutil.Encode(id[:])).First(&modelResult)
if errors.Is(tx.Error, gorm.ErrRecordNotFound) {
return nil, guarddb.ErrNoProvenForID
}

if tx.Error != nil {
return nil, fmt.Errorf("could not get proven")
Comment on lines +71 to +72
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error message for GetPendingProvenByID.

The error message on line 72 should be more descriptive.

- return nil, fmt.Errorf("could not get proven")
+ return nil, fmt.Errorf("could not get pending proven by ID: %w", tx.Error)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if tx.Error != nil {
return nil, fmt.Errorf("could not get proven")
if tx.Error != nil {
return nil, fmt.Errorf("could not get pending proven by ID: %w", tx.Error)

}

qr, err := modelResult.ToPendingProven()
if err != nil {
return nil, err
}
return qr, nil
}
44 changes: 44 additions & 0 deletions services/rfq/guard/guarddb/base/store.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package base

import (
"github.com/synapsecns/sanguine/core/metrics"
listenerDB "github.com/synapsecns/sanguine/ethergo/listener/db"
submitterDB "github.com/synapsecns/sanguine/ethergo/submitter/db"
"github.com/synapsecns/sanguine/ethergo/submitter/db/txdb"
"github.com/synapsecns/sanguine/services/rfq/guard/guarddb"
"gorm.io/gorm"
)

// Store implements the service.
type Store struct {
listenerDB.ChainListenerDB
db *gorm.DB
submitterStore submitterDB.Service
}

// NewStore creates a new store.
func NewStore(db *gorm.DB, metrics metrics.Handler) *Store {
txDB := txdb.NewTXStore(db, metrics)

return &Store{ChainListenerDB: listenerDB.NewChainListenerStore(db, metrics), db: db, submitterStore: txDB}
}

// DB gets the database object for mutation outside of the lib.
func (s Store) DB() *gorm.DB {
return s.db
}

// SubmitterDB gets the submitter database object for mutation outside of the lib.
func (s Store) SubmitterDB() submitterDB.Service {
return s.submitterStore
}

// GetAllModels gets all models to migrate
// see: https://medium.com/@SaifAbid/slice-interfaces-8c78f8b6345d for an explanation of why we can't do this at initialization time
func GetAllModels() (allModels []interface{}) {
allModels = append(txdb.GetAllModels(), &PendingProvenModel{})
allModels = append(allModels, listenerDB.GetAllModels()...)
return allModels
}

var _ guarddb.Service = &Store{}

Check failure on line 44 in services/rfq/guard/guarddb/base/store.go

View workflow job for this annotation

GitHub Actions / Build (1.22.x, ubuntu-latest)

cannot use &Store{} (value of type *Store) as guarddb.Service value in variable declaration: *Store does not implement guarddb.Service (wrong type for method GetPendingProvensByStatus)
Copy link
Contributor

Choose a reason for hiding this comment

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

Implement the missing method GetPendingProvensByStatus.

The Store struct does not fully implement the guarddb.Service interface. It lacks the GetPendingProvensByStatus method.

+ func (s Store) GetPendingProvensByStatus(ctx context.Context, matchStatuses ...guarddb.PendingProvenStatus) ([]guarddb.PendingProven, error) {
+     // Implementation here
+ }

Committable suggestion was skipped due to low confidence.

Tools
GitHub Check: Build (1.22.x, ubuntu-latest)

[failure] 44-44:
cannot use &Store{} (value of type *Store) as guarddb.Service value in variable declaration: *Store does not implement guarddb.Service (wrong type for method GetPendingProvensByStatus)

Loading
Loading