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

cmd, core, eth: add issuance tracking and querying #24723

Closed
wants to merge 7 commits into from

Conversation

karalabe
Copy link
Member

@karalabe karalabe commented Apr 20, 2022

To @JustinDrake

TL;DR

$ nc -U /home/karalabe/.ethereum/goerli/geth.ipc
{"id": 1, "method": "eth_subscribe", "params": ["issuance", 6679436]}

{"block":6679436,"hash":"...","issuance":-39519464,"subsidy":0,"uncles":0,"burn":39519464,"destruct":0}}
{"block":6679437,"hash":"...","issuance":-3461728,"subsidy":0,"uncles":0,"burn":3461728,"destruct":0}}
{"block":6679438,"hash":"...","issuance":-56258184,"subsidy":0,"uncles":0,"burn":56258184,"destruct":0}}
{"block":6679439,"hash":"...","issuance":-22521912,"subsidy":0,"uncles":0,"burn":22521912,"destruct":0}}
{"block":6679440,"hash":"...","issuance":-100726688,"subsidy":0,"uncles":0,"burn":100726688,"destruct":0}}
{"block":6679441,"hash":"...","issuance":-25539160,"subsidy":0,"uncles":0,"burn":25539160,"destruct":0}}
{"block":6679442,"hash":"...","issuance":-40244936,"subsidy":0,"uncles":0,"burn":40244936,"destruct":0}}
{"block":6679443,"hash":"...","issuance":-47638024,"subsidy":0,"uncles":0,"burn":47638024,"destruct":0}}

The PR introduces a new flag --issuance that makes Geth crawl and store the Ether issuance based on the state changes between blocks (i.e. the true value, not what the protocol says).

The user can subscribe on the eth_issuance RPC endpoint from a block in the past to receive all historical and future issuance updates in real time. The returned issuance will not only include the state-crawled issuance, but also the block and uncle subsidy as specified by the protocol as well as the 1559 burn. In certain blocks there will be a tiny discrepancy due to the EVM being able to burn Ether via self destructs. This will also be returned on the API, but it's just the diff between the "measured" and "theoretical" issuance, there's no block tracing done to double check this.

If the issuance was not calculated for a certain block (block was not executed due to snap sync), the issuance and destructed fields in the result will be null, but the other header-calculable fields will be returned nonetheless.

Note, running with issuance tracking will take a performance hit due to having to crawl parts of the trie that would have been left idle otherwise (EVM reads from snapshots), and also will take a disk hit since we need to store a big.Int value for each processed block. There will be no particular effort to optimize these further. This is the cost of issuance tracking.


Whilst the above will give you the issuance across blocks, it will not give you the total Ether supply, unless you full sync and accumulate the data from all blocks. To help with starting tracking from a snap synced node, the PR also introduces a command to crawl the total supply based on Geth's snapshots.

$ geth snapshot crawl-supply --goerli

INFO [05-03|23:52:15.034] Ether supply counting started            block=6,684,848 hash=e19c93..40b9d6 root=6601be..d4c93b
INFO [05-03|23:52:23.035] Ether supply counting in progress        at=a52b63..895fca accounts=6,332,063 supply=80,633,157,448,943,923,314,531,698 elapsed=8.000s
INFO [05-03|23:52:27.243] Ether supply counting complete           block=6,684,848 hash=e19c93..40b9d6 root=6601be..d4c93b accounts=9,814,451 supply=99,998,298,618,046,659,099,131,599 elapsed=12.208s

core/blockchain.go Outdated Show resolved Hide resolved
@holiman
Copy link
Contributor

holiman commented Apr 20, 2022

Question is how @JustinDrake wants to consume the data.

  • It is easy for us to spit out the "issuance/burn" for the entire chain, from genesis until head. (OBS: issued != supply, due to selfdestruct-burns).
  • It is easy to spit out the supply (at a block near the head). However, spitting out the supply for the entire chain requires a block-by-block sync from genesis.

So what would be preferrable?

  1. A super-exact accounting from genesis (slow)
  2. A super-exact accounting from some recent block? (fast)
  3. An issuance/burn chart from genesis, with spot-checks of supply at some recent blocks? (fast)

And would you want the data as a subscription (json) or spat out in some csv form?

@karalabe karalabe changed the title cmd, core, eth: add issuance calculation and logging cmd, core, eth: add issuance tracking and querying May 3, 2022
@JustinDrake
Copy link
Contributor

JustinDrake commented May 4, 2022

@karalabe: Love it! :)

My main suggestion would be to use precise terminology around issuance and supply. Issuance is newly issued ETH, whereas supply is issuance minus burned and self-destroyed ETH. Specific renaming suggestions:

  • --issuance -> --supply_deltas
  • eth_issuance -> supply_deltas
  • issuance -> supply_delta

Other renaming suggestions that I don't feel so strongly about:

  • crawl-supply -> compute-supply
  • subsidy -> fixed_reward (the uncles reward is also a subsidy)
  • uncles -> uncles_reward
  • burn -> fee_burn
  • destruct -> self_destruct

@holiman: The geth snapshot tool combined with block-by-block supply diffs is perfect :)

@JustinDrake
Copy link
Contributor

Another quick comment: I noticed you added an accounts field with geth snapshot which is super helpful. Would it make sense to add an accounts_diff for the RPC as well?

// Issuance calculates the Ether issuance (or burn) across two state tries. In
// normal mode of operation, the expectation is to calculate the issuance between
// two consecutive blocks.
func Issuance(block *types.Block, parent *types.Header, db *trie.Database, config *params.ChainConfig) (*big.Int, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this is not issuance, but "EtherDelta". Issuance is "how much is issued", which IMO should be mining-rewards.

This is how I think of the terms:

  • rewards: mining rewards
  • burn: 1559 burn
  • issuance: rewards - burn
  • delta: issuance - destroyed ether

And the supply is the delta between empty-state (before genesis) and chain head.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this is not issuance, but "EtherDelta". Issuance is "how much is issued", which IMO should be mining-rewards.

Similar comment here :) #24723 (comment)

the delta

"delta" is a great word, probably better than "diff" or "change". I'll edit my comment above to use "delta".

subsidy = ethash.ConstantinopleBlockReward
}
}
// Accumulate the rewards for inclded uncles
Copy link

@alextes alextes Jun 14, 2022

Choose a reason for hiding this comment

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

*included

(:

@alextes
Copy link

alextes commented Jul 5, 2022

Had to rebase this to be able to run it past Gray Glacier. Most of it seemed straightforward, the one change I'd love some more experienced eyes on is how I adapted for #24750. The tries used to calculate the issuance for every block need an owner now. It's hard for me to understand what "an assigned owner for storage proximity." means, that meaning seems captured only in the consuming code, not the defining code and I'm trying not to accidentally become a Geth expert 😅 . I replicated the change that seemed the least invasive, passing the empty Hash. Diff below. Does that look right?

diff --git a/core/issuance/issuance.go b/core/issuance/issuance.go
index 67f75b87a..e6e0e2080 100644
--- a/core/issuance/issuance.go
+++ b/core/issuance/issuance.go
@@ -43,11 +43,11 @@ func Issuance(block *types.Block, parent *types.Header, db *trie.Database, confi
        if block.ParentHash() != parent.Hash() {
                return nil, fmt.Errorf("parent hash mismatch: have %s, want %s", block.ParentHash().Hex(), parent.Hash().Hex())
        }
-       src, err := trie.New(parent.Root, db)
+       src, err := trie.New(common.Hash{}, parent.Root, db)
        if err != nil {
                return nil, fmt.Errorf("failed to open source trie: %v", err)
        }
-       dst, err := trie.New(block.Root(), db)
+       dst, err := trie.New(common.Hash{}, block.Root(), db)
        if err != nil {
                return nil, fmt.Errorf("failed to open destination trie: %v", err)
        }

@alextes
Copy link

alextes commented Jul 5, 2022

Mm, and minor comment (It's hard to not start concerning yourself with adjacent changes 😅 ), this PR removes the DocRoot option and flag. Because of the overhaul to urfave/cli v2 this creates a conflict. It doesn't matter for my work (https://ultrasound.money) at all, but perhaps that change doesn't belong in this PR?

Thought I'd give the heads up in case it's helpful and anyone ever ends up trying to merge this PR 😊 .

@holiman
Copy link
Contributor

holiman commented Sep 23, 2022

@karalabe perhaps close this in favour of #25662 ?

@karalabe
Copy link
Member Author

Closing in favour of the up-to-date-er

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants