-
Notifications
You must be signed in to change notification settings - Fork 47
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
#42: refactor types and utils #86
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package types | ||
|
||
import ( | ||
"errors" | ||
"net/url" | ||
"regexp" | ||
) | ||
|
||
func isValidEthereumAddress(address string) bool { | ||
re := regexp.MustCompile("^0x[0-9a-fA-F]{40}$") | ||
return re.MatchString(address) | ||
} | ||
|
||
func checkIfUrlIsValid(rawUrl string) error { | ||
// Regular expression to validate URLs | ||
urlPattern := regexp.MustCompile(`^(https?|ftp)://[^\s/$.?#].[^\s]*$`) | ||
|
||
// Check if the URL matches the regular expression | ||
if !urlPattern.MatchString(rawUrl) { | ||
return errors.New("invalid url") | ||
} | ||
|
||
parsedURL, err := url.Parse(rawUrl) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// Check if the URL is valid | ||
if parsedURL.Scheme != "" && parsedURL.Host != "" { | ||
return nil | ||
} else { | ||
return errors.New("invalid url") | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,6 @@ | ||
package types | ||
|
||
const EigenPromNamespace = "eigen" | ||
const ( | ||
EigenPromNamespace = "eigen" | ||
MaxNumberOfQuorums = 192 | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,9 +8,9 @@ import ( | |
"math/big" | ||
"net/http" | ||
|
||
"github.com/Layr-Labs/eigensdk-go/crypto/bls" | ||
"github.com/Layr-Labs/eigensdk-go/utils" | ||
"github.com/ethereum/go-ethereum/common" | ||
|
||
"github.com/Layr-Labs/eigensdk-go/crypto/bls" | ||
) | ||
|
||
const ( | ||
|
@@ -32,15 +32,15 @@ type Operator struct { | |
} | ||
|
||
func (o Operator) Validate() error { | ||
if !utils.IsValidEthereumAddress(o.Address) { | ||
if !isValidEthereumAddress(o.Address) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not preserve the Address type as in go-ethereum ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How many times the Validate function will be called? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is a typo at line 22 (addres) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's a good point. this PR is getting stale. might as well do in new PR. I will make an issue. |
||
return errors.New("invalid operator address") | ||
} | ||
|
||
if !utils.IsValidEthereumAddress(o.EarningsReceiverAddress) { | ||
if !isValidEthereumAddress(o.EarningsReceiverAddress) { | ||
return errors.New("invalid EarningsReceiverAddress address") | ||
} | ||
|
||
if o.DelegationApproverAddress != ZeroAddress && !utils.IsValidEthereumAddress(o.DelegationApproverAddress) { | ||
if o.DelegationApproverAddress != ZeroAddress && !isValidEthereumAddress(o.DelegationApproverAddress) { | ||
return fmt.Errorf( | ||
"invalid DelegationApproverAddress address, it should be either %s or a valid non zero ethereum address", | ||
ZeroAddress, | ||
|
@@ -108,21 +108,6 @@ type OperatorAvsState struct { | |
BlockNumber BlockNum | ||
} | ||
|
||
var ( | ||
maxNumberOfQuorums = 192 | ||
) | ||
|
||
func BitmapToQuorumIds(bitmap *big.Int) []QuorumNum { | ||
// loop through each index in the bitmap to construct the array | ||
quorumIds := make([]QuorumNum, 0, maxNumberOfQuorums) | ||
for i := 0; i < maxNumberOfQuorums; i++ { | ||
if bitmap.Bit(i) == 1 { | ||
quorumIds = append(quorumIds, QuorumNum(i)) | ||
} | ||
} | ||
return quorumIds | ||
} | ||
|
||
type QuorumAvsState struct { | ||
QuorumNumber QuorumNum | ||
TotalStake StakeAmount | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,58 +2,12 @@ package utils | |
|
||
import ( | ||
"crypto/ecdsa" | ||
"encoding/json" | ||
"errors" | ||
"log" | ||
"math/big" | ||
"os" | ||
"path/filepath" | ||
"regexp" | ||
|
||
gethcommon "github.com/ethereum/go-ethereum/common" | ||
"github.com/ethereum/go-ethereum/crypto" | ||
"gopkg.in/yaml.v3" | ||
"math/big" | ||
) | ||
|
||
func ReadFile(path string) ([]byte, error) { | ||
shrimalmadhur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
b, err := os.ReadFile(filepath.Clean(path)) | ||
if err != nil { | ||
return nil, err | ||
} | ||
return b, nil | ||
} | ||
|
||
func ReadYamlConfig(path string, o interface{}) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These methods are used in few of libraries we import eigensdk in. So let's not remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aaah makes sense. Also, can we move these functions to a different module like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ideally all internal stuff would've been in an I am not sure what the ideal way of doing this. Are you suggesting we can have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, this might cause breaking changes for the SDK users though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea you are right. the only reason I added is that, multiple repository we have use this function and I just wanted to have it once. but I kind of agree it's on client to have these standard utils on their own. they can choose their own way to do this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @shrimalmadhur if you don't mind can you resolve all the comments which do not need any change? I am a bit confused as to what functions should be added back. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is removed as well in the initial commit ser! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @shrimalmadhur is this good to be merged after we rebase on master? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep. @RohanShrothrium can you rebase and resolve conflicts? and then we can merge this |
||
if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { | ||
log.Fatal("Path ", path, " does not exist") | ||
} | ||
b, err := ReadFile(path) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
err = yaml.Unmarshal(b, o) | ||
if err != nil { | ||
log.Fatalf("unable to parse file with error %#v", err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func ReadJsonConfig(path string, o interface{}) error { | ||
shrimalmadhur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
b, err := ReadFile(path) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
err = json.Unmarshal(b, o) | ||
if err != nil { | ||
log.Fatalf("unable to parse file with error %#v", err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func EcdsaPrivateKeyToAddress(privateKey *ecdsa.PrivateKey) (gethcommon.Address, error) { | ||
publicKey := privateKey.Public() | ||
publicKeyECDSA, ok := publicKey.(*ecdsa.PublicKey) | ||
|
@@ -71,8 +25,3 @@ func RoundUpDivideBig(a, b *big.Int) *big.Int { | |
res.Div(a, b) | ||
return res | ||
} | ||
|
||
func IsValidEthereumAddress(address string) bool { | ||
shrimalmadhur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
re := regexp.MustCompile("^0x[0-9a-fA-F]{40}$") | ||
return re.MatchString(address) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this regex allows invalid uri
https://a.
https://a.com.
https://1.
It also doesn't allow working with ftps, or other uri schema such as grpc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea this is not perfect. as in we tailored it to our requirement and allowed certain endpoints, but definitely can add more.