-
Notifications
You must be signed in to change notification settings - Fork 206
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
Upstream merge up to 1.9.16 #1536
Conversation
Once we detect an invalid transaction during recovering signatures, we should directly exclude this transaction to avoid validating the signatures hereafter. This should optimize the validations times of transactions with invalid signatures to only one time.
* core/vm: use fixed uint256 library instead of big * core/vm: remove intpools * core/vm: upgrade uint256, fixes uint256.NewFromBig * core/vm: use uint256.Int by value in Stack * core/vm: upgrade uint256 to v1.0.0 * core/vm: don't preallocate space for 1024 stack items (only 16) Co-authored-by: Martin Holst Swende <martin@swende.se>
core/state: avoid escape analysis fault when accessing cached state
* eth/downloader tests: fix spurious failing test due to race between receipts/headers * miner tests: fix travis failure on arm64 * eth/downloader: tests - store td in ancients too
… (#21204) * core, eth, internal: extend structLog tracer * core/vm, internal: add storage view * core, internal: add slots to storage directly * core: remove useless * core: address martin's comment * core/vm: fix tests
speicifc -> specific assigened -> assigned frobenious -> frobenius
Co-authored-by: linjing <linjingjing@baidu.com>
core: filter out txs with invalid signatures as soon as possible
…#21220) * fix(freezer): tailId filenum offset were misplaced * core/rawdb: assume first item in freezer always start from zero
* cmd, eth, internal, les: add gasprice cap * cmd/utils, eth: add default value for gasprice cap * all: use txfee cap * cmd, eth: add fix * cmd, internal: address comments
* whisper : use timer.Ticker instead of sleep * lint: Fix linter error Co-authored-by: Guillaume Ballet <gballet@gmail.com>
The ancients variable in the freezer is a list of hashes, which identifies all of the hashes to be frozen. The slice is being allocated with a capacity of `limit`, which is the number of the last block this batch will attempt to add to the freezer. That means we are allocating memory for all of the blocks in the freezer, not just the ones to be added. If instead we allocate `limit - f.frozen`, we will only allocate enough space for the blocks we're about to add to the freezer. On mainnet this reduces usage by about 320 MB.
* common/fdlimit: build on DragonflyBSD * review feedback
p2p: measure packet throughput too, not just bandwidth
These commands mirror the key/URL generation functions of cmd/bootnode. $ devp2p key generate mynode.key $ devp2p key to-enode mynode.key -ip 203.0.113.21 -tcp 30304 enode://78a7746089baf4b8615f54a5f0b67b22b1...
…on (#21203) * crypto/secp256k1: enable use of __int128 This speeds up scalar & field calculations a lot. * crypto/secp256k1: enable endomorphism optimization
* eth: don't block if transaction broadcast loop is returned * eth: kick out peer if we failed to send message * eth: address comment
* core/vm: fix incorrect computation of discount During testing on Yolov1 we found that the way geth calculates the discount is not in line with the specification. Basically what we did is calculate 128 * Bls12381GXMulGas * discount / 1000 whenever we received more than 128 pairs of values. Correct would be to calculate k * Bls12381... for k > 128. * core/vm: better logic for discount calculation * core/vm: better calculation logic, added worstcase benchmarks * core/vm: better benchmarking logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few comments that were answered in different commits, but was part of the way I reviewed the PR (per commit)
Probably will be better to read every comment before answering
@@ -232,8 +232,8 @@ func (t *freezerTable) repair() error { | |||
t.index.ReadAt(buffer, 0) | |||
firstIndex.unmarshalBinary(buffer) | |||
|
|||
t.tailId = firstIndex.offset | |||
t.itemOffset = firstIndex.filenum | |||
t.tailId = firstIndex.filenum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
funny one
@@ -305,8 +305,10 @@ func importChain(ctx *cli.Context) error { | |||
// Import the chain | |||
start := time.Now() | |||
|
|||
var importErr error | |||
for _, arg := range ctx.Args() { | |||
if err := utils.ImportChain(chain, arg); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmm don't actually know what we want to do here, but this will log every error but just return the last one, and if some of the other errors happened, but all the imports were fine, it won't fail. Do we expect that behaviour?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the commit from upstream that brought this change:
413358a
while we could think of a way to maybe improve on it, the change itself is an improvement over the previous version of code that would always return nil even in the face of errors
return common.Hash{}, fmt.Errorf("tx fee (%d wei) exceeds the configured cap (%d wei)", tx.Fee().Uint64(), int64(b.RPCTxFeeCap())) | ||
feeCap := GetWei(b.RPCTxFeeCap()) | ||
if currencyManager.CmpValues(tx.Fee(), tx.FeeCurrency(), feeCap, nil) > 0 { | ||
return common.Hash{}, fmt.Errorf("tx fee (%d celo) exceeds the configured cap (%d celo)", tx.Fee().Uint64(), int64(b.RPCTxFeeCap())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not wei in the message? it could be a stable token, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it can be any currency, yes. What do you think the err message should look like?
// Set up priority client, get its nodeID, increase its balance on the lightServer | ||
prioCli := startClient(t, "prioCli") | ||
defer prioCli.killAndWait() | ||
// 3_000_000_000 once we move to Go 1.13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are even better than that
cmd/geth/les_test.go
Outdated
} | ||
|
||
func TestPriorityClient(t *testing.T) { | ||
t.Skip() // Currently not working |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we think that we should skip this one, or maybe addapt it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it's an extra test, I'd say that whatever road we decide on, we could do that on a separate PR. I don't know the answer though. I think we can create an issue for that (decide & resolve)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created #1557 for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
(Try to rename the file it's not a clique thing anymore)
cmd/geth/testdata/clique.json
Outdated
@@ -8,14 +8,12 @@ | |||
"byzantiumBlock": 0, | |||
"constantinopleBlock": 0, | |||
"petersburgBlock": 0, | |||
"clique": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's not a clique
config anymore, we should probably rename this file
1.19.16 notes
ethstats/ethstats.go conflicts (we have a cherrypicked version). Still was missing a line, added it.
TxFee cap: possible issue with currencies at:
internal/ethapi/api.go SubmitTransaction (RPCTxFeeCap check): instead of the flat comparison brought by upstream, I'm using a currency manager for this
cmd/evm/internal/t8ntool/execution.go:113&114 -> commented to ensure compilation, not sure
what the impact of this is.
Changeset ethereum/go-ethereum@ddeea1e changes comparisons on gas prices, conflicts with our own since we also use feecurrency. I kept our versions of code in some of these (tx_pool.go, tx_list.go)
Skipped new test cmd/geth/les_test.go , may need to modify the configuration of the startup? (priority client test)
Big changes in a commit for tx_list.go and tx_pool.go got reverted in this same PR (This was particularly a pain because resolving the conflicts was hard, only to then resolve the same conflicts again when they had to be undone).