go-ethereum
go-ethereum copied to clipboard
cmd, core, eth: supply delta tracking
By all means, ignore this PR for now and focus on the merge.
This PR is based on #24723 by @karalabe / @holiman , however that PR was getting stale, and https://ultrasound.money needs this code to keep track of various models that take the current ETH supply as an input. We've been running the fork in production for a while, we can't expect you busy gethians to prioritise it now. So here we are. I was planning on using a private fork, some things broke in the rebase, after putting in the effort to properly understand what was going on and fix things, I felt I might as well contribute back the code.
This PR adds the following on top of #24723
- rewrite a commit to fix a seemingly accidentally removed flag (DocRoot).
- rewrite a commit to use new flag fns, some larger shift around updating to urfave/cli/v2 I believe.
- fix various typos.
- update to new
trie.New
interface requesting anowner
arg (please closely review this change, I'll mark it in the diff). - rename many bits as proposed by @JustinDrake and @holiman.
- add a
parentHash
to the supply delta notification.
I'm not married to anything in this PR. Feel free to edit, request edits, propose to drop some of the renamed bits, all fine 👍 .
I don't normally write golang, but apart from 2. and 3. everything logically appears unchanged to my untrained eyes.
Many blessings for the merge core devs 🐼 🙏 ✨ !
🦇🔊
triage discussion: let's try to get this PR into shape, and get it merged.
Rebased against the latest master.
Added three commits to fix code which missed a migration to some new interface. It builds again.
I could work those last three in using rebase if desired, I tried, things got messy, and decided it's not worth my time right now.
I (hastly) rebased this on master and added withdrawal tracking for the shanghai hardfork here: https://github.com/mariusvanderwijden/go-ethereum/tree/supply-delta @alextes could you rebase this and maybe add the withdrawal tracking changes as well? (please don't cherry-pick my commit, the messages are pretty bad ;))
@MariusVanDerWijden ya, they looked a bit messy haha (committing merge conflict markers 😅 ?) I did see you add a newline which is 👌 . Code needs room to breathe 😌 .
Anyway, more on-topic, why calculate withdrawals, log them, but then not emit them? (they aren't in the subscribed notification). Do devs benefit from seeing the logged withdrawals? The latter is really the only case I can think of where this is working as intended 😄 . If yes, 👍 . Otherwise, happy to add them to the notification.
Yeah that code was written 5 minutes before the shanghai hardfork to make sure everything looks okay wrt. issuance. The primary goal was to be able to see the withdrawal amount in the console. If you think its useful to expose that everywhere, please do!
Congrats on getting it in, in that case 😅 .
I have no use for withdrawal notifications, we track withdrawals on the beacon side. I'd suggest leaving the additional functionality out then. Features with zero users are 🙅.
@alextes just checking on this PR - is this something that is still important for your group? I can look into getting this in shape for merge if so.
@lightclient yup, we still run a node based on this branch and to my knowledge anyone that wants to figure out exactly how much ETH is in existence needs this code.
Just curious, how do you utilize this? Do you then have an additional diff where you expose this data over RPC? Or is this command all you need to stop maintaining a fork?
@lightclient it already works as is. You can see an example in the original PR #24723. You add the --supplydelta
flag so deltas are stored for every newly synced block. Then this method existing on the api object at all is enough for one to be able to consume a RPC subscription. These docs describe it. The original PR again has an example using netcat. Not the most obvious way of adding an rpc subscription but it is nice and terse 😄 .
This is ready for review again, cc @karalabe @holiman.
I rebased against master and made the following tweaks:
- added a test in
blockchain_tests.go
- renamed package from
core/supplydelta
tocore/supply
since there were more methods in there than just supply - updated the function signatures a bit for the provided methods in the package
- combined the fixed rewards with the uncle rewards (asked Justin, he is okay with this), and it makes more sense this way to me
- added new
geth_*
rpc namespace to register the method under -- since this is such a geth-specific method this seems to make sense
Manually verified the output against the ultrasound website for both the logs and subscription and the burn matches what is shown there. @alextes if you have the ability to take a look at this branch and make sure it works correctly from your perspective, it would be appreciated!
Triage discussion: put on hold, we want to see if this whole feature can be replicated using the new tracer-features (live chain tracing) and new event-hooks. Then we wouldn't need to have this as a 'native' geth-command.
https://github.com/ethereum/go-ethereum/pull/27629
cc @s1na
Leaving a note I did do another rebase of my own version of this fork for Dencun. Heard from @s1na this may well be the last time I need to 🙌 .
Sidenote: My fork diverged from this one as updated by lightclient. To avoid a DB migration I used a private one.
Guess this is not needed any more with the supply delta on master now.