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.

Sign up for GitHub

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

Jump to bottom

adds evm_client wait for transaction confirmation to QGB #354

Merged
merged 1 commit into from
Apr 26, 2022
Merged

adds evm_client wait for transaction confirmation to QGB #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 55 additions & 6 deletions x/qgb/orchestrator/evm_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"context"
"crypto/ecdsa"
"errors"
"fmt"
"math/big"
"time"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/ethclient"
Expand Down Expand Up @@ -37,20 +39,20 @@ type evmClient struct {
logger tmlog.Logger
wrapper wrapper.QuantumGravityBridge
privateKey *ecdsa.PrivateKey
evmRpc string
evmRPC string
}

func NewEvmClient(
logger tmlog.Logger,
wrapper wrapper.QuantumGravityBridge,
privateKey *ecdsa.PrivateKey,
evmRpc string,
evmRPC string,
) EVMClient {
return &evmClient{
logger: logger,
wrapper: wrapper,
privateKey: privateKey,
evmRpc: evmRpc,
evmRPC: evmRPC,
}
}

Expand All @@ -60,6 +62,7 @@ func (ec *evmClient) UpdateValidatorSet(
valset types.Valset,
sigs []wrapper.Signature,
) error {
ec.logger.Info(fmt.Sprintf("relaying valset %d...", nonce))
opts, err := ec.NewTransactOpts(ctx, 1000000)
if err != nil {
return err
Expand All @@ -86,7 +89,22 @@ func (ec *evmClient) UpdateValidatorSet(
if err != nil {
return err
}
ec.logger.Info("ValSetUpdate", tx.Hash().String())

// TODO put this in a separate function and listen for new EVM blocks instead of just sleeping
for i := 0; i < 60; i++ {
Copy link
Member

Choose a reason for hiding this comment

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

Why the magic 60?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was 6 at the beginning to wait for a minute. However, I am testing against rinkeby/ropsten, and they take a lot of time to include transactions in blocks. So, I changed it to 10 minutes of wait time.
In the future, when we have the worker pool design, we will change this by listening to new blocks and waiting for a number of confirmations before moving to the next attestation

Copy link
Member Author

@rach-id rach-id Apr 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This is a QGB contract in action:
https://ropsten.etherscan.io/address/0x820a20eb3c8f7789557595874b89dd71fea11a0e
I am generating a new valset every 15 Celestia blocks.
Don't mind the failing transactions, as those are duplicates (will be fixed when we implement the new design)

PS: We still don't use the latest nonce => height change, and we're deploying an older version

ec.logger.Debug(fmt.Sprintf("waiting for valset %d to be confirmed: %s", nonce, tx.Hash().String()))
lastNonce, err := ec.StateLastValsetNonce(&bind.CallOpts{Context: ctx})
if err != nil {
return err
}
if lastNonce == nonce {
ec.logger.Info(fmt.Sprintf("relayed valset %d: %s", nonce, tx.Hash().String()))
return nil
}
time.Sleep(10 * time.Second)
}
Comment on lines +93 to +105
Copy link
Member

Choose a reason for hiding this comment

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

as stated in the todo, I like the idea of a separate evm client method that waits for a specific update or something similar

Copy link
Member

Choose a reason for hiding this comment

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

let's write that down in an issue if we haven't already tho

Copy link
Member Author

Choose a reason for hiding this comment

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

We can create a generic method that subscribes to new blocks and checks if a transaction hash is included or not.
Could be something like:

func (ec *EvmClient) waitForConfirmations(txHash string, numberOfConfirmations int) error

I wrote some snippets, but it would take much time debugging and making it work. Especially that subscribing to events requires using a web socket connection and that needs to be added to the config which itself needs to be updated to reflect the different commands.
Thus, I decided to leave it for later and do this instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

issue: #356

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me know if we can merge.

Copy link
Member

Choose a reason for hiding this comment

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

:shipit:


ec.logger.Error(fmt.Sprintf("failed valset %d: %s", nonce, tx.Hash().String()))
return nil
}

Expand Down Expand Up @@ -118,14 +136,45 @@ func (ec *evmClient) SubmitDataRootTupleRoot(
if err != nil {
return err
}
ec.logger.Info("DataRootTupleRootUpdated", tx.Hash().String())

// TODO put this in a separate function and listen for new EVM blocks instead of just sleeping
for i := 0; i < 60; i++ {
ec.logger.Debug(fmt.Sprintf(
"waiting for data commitment %d-%d to be confirmed: %s",
lastDataCommitmentNonce,
lastDataCommitmentNonce-types.DataCommitmentWindow,
tx.Hash().String(),
))
lastNonce, err := ec.StateLastDataRootTupleRootNonce(&bind.CallOpts{Context: ctx})
if err != nil {
return err
}
if lastNonce == lastDataCommitmentNonce {
ec.logger.Info(fmt.Sprintf(
"relayed data commitment %d-%d: %s",
lastDataCommitmentNonce,
lastDataCommitmentNonce-types.DataCommitmentWindow,
tx.Hash().String(),
))
return nil
}
time.Sleep(10 * time.Second)
}
ec.logger.Error(
fmt.Sprintf(
"failed to relay data commitment %d-%d: %s",
lastDataCommitmentNonce,
lastDataCommitmentNonce-types.DataCommitmentWindow,
tx.Hash().String(),
),
)
return nil
}

func (ec *evmClient) NewTransactOpts(ctx context.Context, gasLim uint64) (*bind.TransactOpts, error) {
builder := newTransactOptsBuilder(ec.privateKey)

ethClient, err := ethclient.Dial(ec.evmRpc)
ethClient, err := ethclient.Dial(ec.evmRPC)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion x/qgb/orchestrator/relayer_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (oc *relayerClient) SubscribeValset(ctx context.Context) (<-chan types.Vals
continue
}
valsetsChan <- *newVs
// Give some time for newVs to be commited before we continue
// Give some time for newVs to be committed before we continue
// This will change with the worker pool design pattern we will adopt
time.Sleep(10 * time.Second)
}
Expand Down