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

L2Wallet SendTransaction is not thread safe #42

Open
Meteriox opened this issue Apr 1, 2024 · 4 comments
Open

L2Wallet SendTransaction is not thread safe #42

Meteriox opened this issue Apr 1, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@Meteriox
Copy link

Meteriox commented Apr 1, 2024

I found that when multiple goroutines concurrently call SendTransaction to send transactions, it's possible to get the same txhash for different transactions. This can lead to some transactions not being confirmed, and calling WaitMined after sending the transaction can't retrieve the transaction information, resulting in a deadlock. The code to reproduce this issue is as follows:
`func main() {
PrivateKey := ""
ZkSyncEraProvider := "
"

client, err := clients.Dial(ZkSyncEraProvider)
if err != nil {
	return
}
defer client.Close()

wallet, err := accounts.NewWalletL2(common.Hex2Bytes(PrivateKey), &client)
if err != nil {
	return
}

SatsAddress := common.HexToAddress("***")
ZBTCAddress := common.HexToAddress("***")
l2address := common.HexToAddress("***")

contractABI, err := abi.JSON(strings.NewReader(erc20ABI))
if err != nil {
	log.Fatal(err)
}

var wg sync.WaitGroup
wg.Add(3) 

go func() {
	defer wg.Done()
	amount := ToWei("1", 18)
	mintData, err := contractABI.Pack("mint", l2address, amount)
	if err != nil {
		return
	}
	hash, err := wallet.SendTransaction(context.Background(), &accounts.Transaction{
		To:   &SatsAddress,
		Data: mintData,
	})
	if err != nil {
		fmt.Println(err)
		return
	}
	_, err = client.WaitMined(context.Background(), hash)
	if err != nil {
		log.Panic(err)
	}
}()

go func() {
	defer wg.Done()
	amount := ToWei("0.001", 18)
	mintData, err := contractABI.Pack("mint", l2address, amount)
	if err != nil {
		return
	}
	hash, err := wallet.SendTransaction(context.Background(), &accounts.Transaction{
		To:   &ZBTCAddress,
		Data: mintData,
	})
	if err != nil {
		fmt.Println(err)
		return
	}
	_, err = client.WaitMined(context.Background(), hash)
	if err != nil {
		log.Panic(err)
	}
}()

go func() {
	defer wg.Done()
	amount := ToWei("0.001", 18)
	mintData, err := contractABI.Pack("mint", l2address, amount)
	if err != nil {
		return
	}
	hash, err := wallet.SendTransaction(context.Background(), &accounts.Transaction{
		To:   &ZBTCAddress,
		Data: mintData,
	})
	if err != nil {
		fmt.Println(err)
		return
	}
	_, err = client.WaitMined(context.Background(), hash)
	if err != nil {
		log.Panic(err)
	}
}()
wg.Wait()

}`

However, after adding a lock to SendTransaction, a new problem arose.
Since the Nonce calculation in PopulateTransaction is based on an RPC call rather than local caching, concurrent calls to SendTransaction can lead to the Nonce not being updated in time.
This can result in transactions with the same Nonce field being generated. When the previous transaction is submitted and successfully processed, the subsequent transaction will be rejected due to an invalid Nonce field.

go func() { defer wg.Done() amount := ToWei("1", 18) mintData, err := contractABI.Pack("mint", l2address, amount) if err != nil { return } lock.Lock() hash, err := wallet.SendTransaction(context.Background(), &accounts.Transaction{ To: &SatsAddress, Data: mintData, }) if err != nil { fmt.Println(err) return } lock.Unlock() _, err = client.WaitMined(context.Background(), hash) if err != nil { log.Panic(err) } }()

In the end, I had to include WaitMined in the locked scope as well, forcing the originally concurrent transactions to be executed and confirmed in a completely serial manner.

go func() { defer wg.Done() amount := ToWei("1", 18) mintData, err := contractABI.Pack("mint", l2address, amount) if err != nil { return } lock.Lock() hash, err := wallet.SendTransaction(context.Background(), &accounts.Transaction{ To: &SatsAddress, Data: mintData, }) if err != nil { fmt.Println(err) return } _, err = client.WaitMined(context.Background(), hash) if err != nil { log.Panic(err) } lock.Unlock() }()

My question is, when using zksync-gosdk, is it only possible to execute transactions serially, and not to send transactions concurrently?
If concurrent transactions are possible, how can it be done? If not, please add a note in the comments of SendTransaction indicating that it is not thread-safe for concurrent use.

@Meteriox Meteriox added the bug Something isn't working label Apr 1, 2024
@danijelTxFusion
Copy link
Contributor

danijelTxFusion commented Apr 2, 2024

Method WalletL2.sendTransaction is not implemented to be thread-safe. If there is a need by the community to make it thread-safe, then it will be implemented.

@Meteriox
Copy link
Author

Meteriox commented Apr 8, 2024

Method WalletL2.sendTransaction is not implemented to be thread-safe. If there is a need by the community to make it thread-safe, then it will be implemented.

If my requirement now is to support concurrency safety when calling a contract to send transactions, which interface or function should I use to implement it? Or should I directly use the rpc method of the node to implement it?

@danijelTxFusion
Copy link
Contributor

If my requirement now is to support concurrency safety when calling a contract to send transactions, which interface or function should I use to implement it? Or should I directly use the rpc method of the node to implement it?

You can call directlyeth_sendTransaction RPC method. Here are the steps:

  • Prepare your transaction manually with correct nonce (similar like this)
  • Signs the prepared transaction using Signer.
  • Send signed transaction using Client.sendRawTransaction.
client, err := clients.Dial("https://sepolia.era.zksync.dev")
if err != nil {
	log.Panic(err)
}
defer client.Close()

chainID, err := client.ChainID(context.Background())
if err != nil {
    log.Panic(err)
}
baseSigner, err := NewBaseSignerFromRawPrivateKey(rawPrivateKey, chainID.Int64())
if err != nil {
    log.Panic(err)
}
signer := Signer(baseSigner)

var preparedTx zkTypes.Transaction712 // prepared tx in thread-safe manner

signature, err := signer.SignTypedData(signer.Domain(), preparedTx)
if err != nil {
	log.Panic(err)
}
tx, err := preparedTx.RLPValues(signature)
if err != nil {
	log.Panic(err)
}
hahs, err := client.SendRawTransaction(ctx context.Context, tx)
if err != nil {
	log.Panic(err)
}

@Meteriox Meteriox closed this as completed Apr 9, 2024
@Meteriox Meteriox reopened this Apr 11, 2024
@Meteriox
Copy link
Author

If my requirement now is to support concurrency safety when calling a contract to send transactions, which interface or function should I use to implement it? Or should I directly use the rpc method of the node to implement it?

You can call directlyeth_sendTransaction RPC method. Here are the steps:

  • Prepare your transaction manually with correct nonce (similar like this)
  • Signs the prepared transaction using Signer.
  • Send signed transaction using Client.sendRawTransaction.
client, err := clients.Dial("https://sepolia.era.zksync.dev")
if err != nil {
	log.Panic(err)
}
defer client.Close()

chainID, err := client.ChainID(context.Background())
if err != nil {
    log.Panic(err)
}
baseSigner, err := NewBaseSignerFromRawPrivateKey(rawPrivateKey, chainID.Int64())
if err != nil {
    log.Panic(err)
}
signer := Signer(baseSigner)

var preparedTx zkTypes.Transaction712 // prepared tx in thread-safe manner

signature, err := signer.SignTypedData(signer.Domain(), preparedTx)
if err != nil {
	log.Panic(err)
}
tx, err := preparedTx.RLPValues(signature)
if err != nil {
	log.Panic(err)
}
hahs, err := client.SendRawTransaction(ctx context.Context, tx)
if err != nil {
	log.Panic(err)
}

i have tried the above mentioned method to send raw transaction concurrently, but i have found that i need to mange tx.nonce locally which is difficult for me, so please implement the wallet sendtx method to be thread safety,thks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants