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

Code sync to go-ethereum v1.13.14 #1152

Draft
wants to merge 43 commits into
base: master
Choose a base branch
from
Draft

Code sync to go-ethereum v1.13.14 #1152

wants to merge 43 commits into from

Conversation

darioush
Copy link
Collaborator

Why this should be merged

Takes code from upstream

How this works

Takes code from upstream

How this was tested

[ ] CI
[ ] testnet

How is this documented

N/A


log.Info("Closing quit channel")
close(bc.quit)
close(bc.quit) // XXX: Why is this closed before [scope] but it's opposite upstream?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why is bc.quit closed before scope but it's opposite upstream?

"bytes"
_ "embed"
"encoding/hex"
_ "embed" // XXX: why is this import here?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is this needed?

PrivateKey []byte `json:"secretKey,omitempty"` // for tests
}

// XXX: why is BaseFee/Alloc ordered differently compared to upstream?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does the different ordering matter? if not should we put it back?

)

// TODO: simplify the unindexer logic and this test.
// XXX: These tests are moved from blockchain_test.go here.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this test was deleted upstream, but it's the one we modified.
maybe we should move this to txindexer_extra_test.go?

stateObject := s.getStateObject(addr)
if stateObject != nil {
return stateObject.Balance()
}
return new(big.Int).Set(common.Big0)
return common.U2560 // XXX: verify we don't need to make a copy of this
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

probably we should make a copy of this to be safe

// AdjustTime changes the block timestamp and creates a new block.
// It can only be called on empty blocks.
func (n *Backend) AdjustTime(adjustment time.Duration) error {
_, err := n.buildBlock(false, uint64(adjustment)) // XXX: shouldn't the granularity be in seconds?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this doesn't seem right to me, block timestamps should be in seconds?

@@ -213,7 +213,8 @@ func (c *ChainConfig) Description() string {
}

banner += "Hard forks (timestamp based):\n"
banner += fmt.Sprintf(" - Cancun Timestamp: @%-10v (https://github.com/ava-labs/avalanchego/releases/tag/v1.12.0)\n", ptrToString(c.CancunTime))
banner += fmt.Sprintf(" - Cancun Timestamp: @%-10v (https://github.com/ava-labs/avalanchego/releases/tag/v1.12.0)\n", ptrToString(c.CancunTime)) /// XXX: should we link the ethereum execution spec here instead
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cosmetic issue, but we can link the ethereum spec here

@@ -117,7 +118,8 @@ func mintNativeCoin(accessibleState contract.AccessibleState, caller common.Addr
stateDB.CreateAccount(to)
}

stateDB.AddBalance(to, amount)
amountU256, _ := uint256.FromBig(amount) // XXX: should we check overflow?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

^

bigIntAmount := (*big.Int)(amount)
state.AddBalance(to, bigIntAmount)
amountBig := (*big.Int)(amount)
amountU256, _ := uint256.FromBig(amountBig) // XXX: should we check overflow?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

^

@@ -29,7 +30,8 @@ func upgradeAccount(account common.Address, upgrade params.StateUpgradeAccount,
}

if upgrade.BalanceChange != nil {
state.AddBalance(account, (*big.Int)(upgrade.BalanceChange))
balanceChange, _ := uint256.FromBig((*big.Int)(upgrade.BalanceChange)) // XXX: do we need to check overflow?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

^

Comment on lines +109 to +110
// NonceAt retrieves the nonce associated with an account.
NonceAt(ctx context.Context, account common.Address, blockNum *big.Int) (uint64, error)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

need to be able to build on top of the nonce in the txpool, not the accepted nonce.

Base automatically changed from geth-v1.13.8 to master May 16, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog 🗄️
Development

Successfully merging this pull request may close these issues.

None yet

1 participant