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

Fix lint errors #763

Closed
wants to merge 8 commits into from
Closed

Fix lint errors #763

wants to merge 8 commits into from

Conversation

hubchub
Copy link
Contributor

@hubchub hubchub commented Apr 11, 2023

@dominant-strategies/core-dev

@@ -20,7 +20,7 @@ import (
"context"
"sync"

"github.com/dominant-strategies/go-quai"
ethereum "github.com/dominant-strategies/go-quai"
Copy link
Member

@wizeguyy wizeguyy Apr 12, 2023

Choose a reason for hiding this comment

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

I don't understand this addition. Any idea why the linter added this?

Copy link
Member

Choose a reason for hiding this comment

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

bump on this

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah looks strange, maybe the "name" of our package is ethereum somewhere still?
They implemented this auto alias feature golangci/golangci-lint#2401

@@ -1047,7 +1047,7 @@ func (d *Downloader) processHeaders(origin uint64) error {
if rollback > 0 {
curBlock := d.core.CurrentBlock().NumberU64()
log.Warn("Rolled back chain segment",
"block", fmt.Sprintf("%d->%d", curBlock), "reason", rollbackErr)
"block", fmt.Sprintf("%d->%d", curBlock, rollbackErr), "reason")
Copy link
Member

Choose a reason for hiding this comment

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

Neither of these print statements look correct, lol. I don't think this is the linters fault, but just want to check with you first

@wizeguyy
Copy link
Member

@hubchub what is the status on this? IIRC you were adding some of the tests back in

@hubchub
Copy link
Contributor Author

hubchub commented Apr 14, 2023

reintroduced the tests and rebased on main

the linter is failing but only on test files

tc.evictedVals = append(tc.evictedVals, v)
}
// outside critical section
//func (tc *TimedCache) onEvicted(k, v interface{}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this function isn't being used then we should probably remove it

for i, blockHash := range pendingBlockBody.SubManifest {
subManifest[i] = blockHash
}
copy(subManifest, pendingBlockBody.SubManifest)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just seeing this so often I wonder if there was any reason we manually set each element instead of using copy. @gameofpointers any thoughts?

// and delete all the data associated with the snapshot (accounts, storage,
// metadata). After all is done, the snapshot range of the database is compacted
// to free up unused data blocks.
func wipeSnapshot(db ethdb.KeyValueStore, full bool) chan struct{} {
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably will need this code relatively soon. Still delete but bringing it to everyone's attention @dominant-strategies/core-dev

@@ -541,6 +541,7 @@ func (pool *TxPool) TxPoolPending(enforceTips bool, etxSet types.EtxSet) (map[co
}
}

//nolint
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for this?

@@ -29,7 +29,7 @@ import (
"github.com/dominant-strategies/go-quai/crypto/bn256"
"github.com/dominant-strategies/go-quai/params"

//lint:ignore SA1019 Needed for precompile
//nolint SA1019 Needed for precompile
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still an issue?

@@ -284,7 +284,7 @@ func (evm *EVM) CallCode(caller ContractRef, addr common.Address, input []byte,
// The contract is a scoped environment for this execution context only.
contract := NewContract(caller, AccountRef(caller.Address()), value, gas)
contract.SetCallCode(&addrCopy, evm.StateDB.GetCodeHash(internalAddr), evm.StateDB.GetCode(internalAddr))
ret, err = evm.interpreter.Run(contract, input, false)
ret, _ = evm.interpreter.Run(contract, input, false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these need error checking blocks?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this whole blake2b directory anymore

@@ -20,7 +20,7 @@ import (
"context"
"sync"

"github.com/dominant-strategies/go-quai"
ethereum "github.com/dominant-strategies/go-quai"
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah looks strange, maybe the "name" of our package is ethereum somewhere still?
They implemented this auto alias feature golangci/golangci-lint#2401

@wizeguyy
Copy link
Member

wizeguyy commented May 4, 2023

closing this for now. We can reopen when we're ready to restart this effort

@wizeguyy wizeguyy closed this May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants