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

Concurrent version of PedersenHash isn't working #7

Closed
emil14 opened this issue Apr 4, 2023 · 3 comments
Closed

Concurrent version of PedersenHash isn't working #7

emil14 opened this issue Apr 4, 2023 · 3 comments

Comments

@emil14
Copy link

emil14 commented Apr 4, 2023

Is this library still works? Could you please admit it? It looks like I didn't change anything in my code but it's not working any longer.

This is how I use it

	orderSignParam := starkex.OrderSignParam{
		NetworkId:  r.networkID, // 3
		PositionId: r.positionID, // got from getAccount()
		Market:     req.Market, // "ETH-USD"
		Side:       "BUY",
		HumanSize:  req.Size, // 1
		HumanPrice: req.Price, // 1
		LimitFee:   req.LimitFee, // "0.000000"
		ClientId:   req.ClientId, // "1" 
		Expiration: expiration,  // "2023-04-05T15:50:15.285Z"
	}

	signature, err := starkex.OrderSign(r.starkexCreds.PrivateKey, orderSignParam) // FIXME invaid signature
	if err != nil {
		return fmt.Errorf("starkex sing: %w", err)
	}
  • Not sure about req.ClientId, // "1" though
  • r.starkexCreds.PrivateKey is 100% right (checked about 5 times with local-storage on dydx website)

UPD: clientID if generated now, it wasn't the problem

I think you last commit where you've added concurrency to crypto stuff broke something. Rollback to the prev version fixed the problem

@emil14 emil14 changed the title DYDX "invalid signature for order" 400 response Concurrent version of PedersenHash isn't working Apr 7, 2023
@tpunt
Copy link

tpunt commented Jun 2, 2023

At a quick glance, it looks as though the wg.Add(1) (on line 43) needs to happen outside of the goroutine creation in commit a654684 @yanue

@tpunt
Copy link

tpunt commented Jun 3, 2023

Actually, I don't see how making the PedersenHash function concurrent like this could work anyway...

Also, the extended Euclidean algorithm implementation (igcdex()) looks broken. E.g. the first if statement is comparing the same conditions.

I've forked the project for now (tpunt/starkex) to apply the fixes, since the owner is not currently around. Happy to create PRs to fix things in this repo when the owner is back (I really don't want to maintain a fork of this).

@emil14
Copy link
Author

emil14 commented Jun 5, 2023

Awesome! Thank you so much @tpunt

@yanue yanue closed this as completed Jul 4, 2023
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

No branches or pull requests

3 participants