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

Added a guradian module. #82

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Added a guradian module. #82

wants to merge 2 commits into from

Conversation

evan57-liu
Copy link

Used to filter transactions from sanctioned addresses.

FilterFilePath string // File path to the bloom filter file
PrivateKeyPath string // Path to the private key file
PublicKeyPath string // Path to the public key file
Passphrase string // Passphrase for the private key
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these required? If not, better remove them to decrease operational complexity

Copy link
Author

Choose a reason for hiding this comment

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

Done


// Reset allows you to reset the Guardian instance.
// This stops the current instance and resets it to nil.
func Reset() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume Reset should only be called in unit tests, right?
If so, move this function to test dir.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@@ -1241,6 +1242,14 @@ func (p *BlobPool) add(tx *types.Transaction) (err error) {
addtimeHist.Update(time.Since(start).Nanoseconds())
}(time.Now())

// When the guardian module is enabled, check if the sender or recipient in the
// transaction is in the filter file
if instance, err := guardian.GetInstance(); err == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for filtering blob transactions as eip4844 is not enabled in story right now

Copy link
Author

Choose a reason for hiding this comment

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

Done

// GetInstance returns the singleton Guardian instance, or an error if it is not initialized.
func GetInstance() (*Guardian, error) {
if instance == nil {
return nil, errors.New("guardian is not initialized")
Copy link
Contributor

Choose a reason for hiding this comment

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

not initialized is not an error while guardian mode is disabled

Copy link
Author

Choose a reason for hiding this comment

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

Done


// checkAddress checks if the given address is in the filter list.
func (p *Guardian) checkAddress(tx *types.Transaction, from, addr string) (bool, error) {
ok, err := p.filter.CheckAddress(strings.ToLower(addr))
Copy link
Contributor

Choose a reason for hiding this comment

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

here the check is case insensitive

Copy link
Author

Choose a reason for hiding this comment

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

Done

Used to filter transactions from sanctioned addresses.
// newGuardian creates a new Guardian instance from the provided config.
func newGuardian(config Config) (*Guardian, error) {
// Create the secure data handler
handler, err := securedata.NewPGPSecureHandler(
Copy link
Contributor

Choose a reason for hiding this comment

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

we prefer not using secureHandler currently.
let's delete all related code and test without secureHandler

func InitInstance(config Config) {
initOnce.Do(func() {
if !config.Enabled {
log.Warn("Guardian is disabled")
Copy link
Contributor

Choose a reason for hiding this comment

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

As Guardian mode is disabled by default, this log should be info level.
Better output a log whether it's disabled or enabled.

// transaction is in the filter file
if instance := guardian.GetInstance(); instance != nil {
if instance.CheckTransaction(pool.signer, tx) {
validTxMeter.Mark(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

we should not use this metric since it's for valid transaction.
I suggest to create a new metric likesanctionedTx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants