Skip to content

core/txpool: drop the zero balance #27833

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

Closed
wants to merge 1 commit into from
Closed

core/txpool: drop the zero balance #27833

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
3 changes: 3 additions & 0 deletions core/txpool/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ func ValidateTransactionWithState(tx *types.Transaction, signer types.Signer, op
balance = opts.State.GetBalance(from)
cost = tx.Cost()
)
if balance.Sign() == 0 {
Copy link
Member

@rjl493456442 rjl493456442 Aug 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I guess this additional clause can be incorrect if 1559 isn't activated in some private chains?

Personally I will prefer to not have it. It's more like a check from chain's perspective(basefee must cost something), but not from the txpool's perspective(enough funds to cover the expense).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds reasonable. Let me see if I can check with eip1559 here or check it outside

Copy link
Contributor

Choose a reason for hiding this comment

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

Historically, it has been possible to send transactions from empty accounts, but we (geth) don't like it and have tried to prevent it (but it's been allowed by consensus). I think adding this check is in line with our previous stance, that we try to avoid it. It leads to unexpected quirks.

Copy link
Member

Choose a reason for hiding this comment

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

Well, it's kind of impossible to set a 0 gas price on our pool, so the balance check in the next line will fail. I'm not a fan of adding checks for stuff that cannot happen really.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@karalabe but it did happen in the Goerli and Sepolia testnet. see the reproduce script in #27833 (comment)

and here is a simple sepolia docker-compose

version: '3'

services:
  el:
    image: ethereum/client-go:v1.12.0
    container_name: sepo-el
    stop_signal: SIGINT
    stop_grace_period: 2m
    deploy:
      resources:
        limits:
          memory: 8G
        reservations:
          memory: 2G
    healthcheck:
      test: ['CMD-SHELL', 'geth attach --exec eth.blockNumber /data/geth.ipc']
      interval: 10s
      timeout: 5s
      retries: 5
    command:
      - '--sepolia'
      - '--syncmode=snap'
      - '--db.engine=pebble'
      - '--datadir=/data'
      - '--datadir.ancient=/ancient'
      - '--cache.preimages'
      - '--cache.database=60'
      - '--cache.trie=30'
      - '--cache.gc=5'
      - '--cache.snapshot=5'
      - '--cache.trie.rejournal=48h'
      - '--maxpeers=100'
      - '--port=30303'
      - '--http'
      - '--http.addr=0.0.0.0'
      - '--http.port=4545'
      - '--http.api=engine,web3,eth,net,personal,miner,txpool,debug,admin'
      - '--ws'
      - '--ws.addr=0.0.0.0'
      - '--ws.port=4546'
      - '--ws.origins=*'
      - '--ws.api=engine,web3,eth,net,personal,miner,txpool,debug,admin'
      - '--graphql'
      - '--graphql.vhosts=*'
      - '--graphql.corsdomain=*'
      - '--authrpc.addr=0.0.0.0'
      - '--authrpc.port=8551'
      - '--authrpc.vhosts=*'
      - '--authrpc.jwtsecret=/jwt/jwt.hex'
      - '--http.vhosts=*'
      - '--http.corsdomain=*'
      - '--metrics'
      - '--metrics.expensive'
      - '--metrics.addr=0.0.0.0'
      - '--metrics.port=6040'
      - '--pprof'
      - '--pprof.addr=0.0.0.0'
      - '--pprof.port=6041'
      - '--txlookuplimit=0'
      - '--txpool.accountslots=32'
      - '--txpool.globalslots=1024'
      - '--txpool.accountqueue=256'
      - '--txpool.globalqueue=2048'
    volumes:
      - ./data:/data
      - /data/sepo-ancient:/ancient
      - ./jwt:/jwt
    restart: unless-stopped
    ports:
      - '5500:30303/tcp'
      - '5500:30303/udp'
      - '5545:4545'
      - '5546:4546'
      - '5040:6040'
      - '5041:6041'
      - '5551:8551'

  cl:
    image: prysmaticlabs/prysm-beacon-chain:v4.0.7
    container_name: sepo-cl
    stop_grace_period: 2m
    deploy:
      resources:
        limits:
          memory: 4G
        reservations:
          memory: 1G
    command:
      - '--accept-terms-of-use'
      - '--sepolia'
      - '--datadir=/data'
      - '--rpc-host=0.0.0.0'
      - '--rpc-port=9548'
      - '--p2p-tcp-port=13005'
      - '--p2p-udp-port=12005'
      - '--monitoring-host=0.0.0.0'
      - '--execution-endpoint=http://el:8551'
      - '--jwt-secret=/jwt/jwt.hex'
      - '--grpc-gateway-host=0.0.0.0'
      - '--grpc-gateway-port=3505'
      - '--grpc-gateway-corsdomain=*'
      - '--checkpoint-sync-url=https://sepolia.beaconstate.info'
      - '--genesis-beacon-api-url=https://sepolia.beaconstate.info'
    volumes:
      - ./prysm:/data
      - ./jwt:/jwt
    restart: unless-stopped

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, it's kind of impossible to set a 0 gas price on our pool,

I guess that check is not performed for local transactions, @karalabe ?

return fmt.Errorf("%w: balance 0", core.ErrInsufficientFunds)
}
if balance.Cmp(cost) < 0 {
return fmt.Errorf("%w: balance %v, tx cost %v, overshot %v", core.ErrInsufficientFunds, balance, cost, new(big.Int).Sub(cost, balance))
}
Expand Down