Skip to content

fix: pp init in PDPV0 #661

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

Open
wants to merge 1 commit into
base: pdpv0
Choose a base branch
from
+5 −2
Open

fix: pp init in PDPV0 #661

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
7 changes: 5 additions & 2 deletions tasks/pdp/task_init_pp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"math/big"
"strings"

"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/ethereum/go-ethereum/core/types"
Expand Down Expand Up @@ -191,6 +192,8 @@ func (ipp *InitProvingPeriodTask) Do(taskID harmonytask.TaskID, stillOwned func(
return false, xerrors.Errorf("failed to send transaction: %w", err)
}

txHashLower := strings.ToLower(txHash.Hex())
Copy link

Choose a reason for hiding this comment

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

I am not a gopher, but some quick snooping through source seems to show that txHash.Hex() should always produce lowercase hex strings?

txHash.Hex() definition seems to be here:

https://github.com/ethereum/go-ethereum/blob/ea28346f91b65c9882e942d2fcad9cdbaa09d706/common/types.go#L86C1-L87C1

which uses go-ethereum hex util:

https://github.com/ethereum/go-ethereum/blob/ea28346f91b65c9882e942d2fcad9cdbaa09d706/common/hexutil/hexutil.go#L83-L88

which uses encoding/hex stdlib's Encode, which should only ever produce lowercase based on the hextable="0123456789abcdef"? https://cs.opensource.google/go/go/+/refs/tags/go1.23.2:src/encoding/hex/hex.go;l=17

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had the same understanding but I remember that DB was complaining about serialization. This lead to basically ppInit never properly working. Curio keept spawning new ppInit tasks. If that is not the case anymore with the pdpv0 branch then feel free to close this. I don't have the error handy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems we were focusing on different things. Actual fix is .Hex(). strings.ToLower() is there only due to historical context. Perhaps, we can clean that up in other places as well later.

Copy link
Member

Choose a reason for hiding this comment

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

I think maybe it's just addresses we need to be careful with, not necessarily transaction hashes. I'd still be more comfortable doing a tolower on all of these things just to be sure. EIP-55 shows up in weird places so I trust none of this.


// Update the database in a transaction
_, err = ipp.db.BeginTransaction(ctx, func(tx *harmonydb.Tx) (bool, error) {
// Update pdp_data_sets
Expand All @@ -200,7 +203,7 @@ func (ipp *InitProvingPeriodTask) Do(taskID harmonytask.TaskID, stillOwned func(
prev_challenge_request_epoch = $2,
prove_at_epoch = $3
WHERE id = $4
`, txHash.Hex(), ts.Height(), init_prove_at.Uint64(), dataSetId)
`, txHashLower, ts.Height(), init_prove_at.Uint64(), dataSetId)
if err != nil {
return false, xerrors.Errorf("failed to update pdp_data_sets: %w", err)
}
Expand All @@ -212,7 +215,7 @@ func (ipp *InitProvingPeriodTask) Do(taskID harmonytask.TaskID, stillOwned func(
_, err = tx.Exec(`
INSERT INTO message_waits_eth (signed_tx_hash, tx_status)
VALUES ($1, 'pending') ON CONFLICT DO NOTHING
`, txHash.Hex())
`, txHashLower)
if err != nil {
return false, xerrors.Errorf("failed to insert into message_waits_eth: %w", err)
}
Expand Down