Add Decoder for The Graph staking #6712

d347h-eth · 2023-10-06T13:38:02Z

Related to #2044

For Delegator staking the typical scenario is:

call delegate() (emits StakeDelegated event)
call undelegate() (emits StakeDelegatedLocked event)
call withdrawDelegated() (emits StakeDelegatedWithdrawn event)

CLAassistant · 2023-10-06T13:38:08Z

All committers have signed the CLA.

rotkehlchen/chain/ethereum/modules/the_graph/constants.py

rotkehlchen/chain/ethereum/modules/the_graph/decoder.py

codecov · 2023-10-06T14:20:35Z

Codecov Report

Merging #6712 (61099ca) into develop (1c3e052) will decrease coverage by 0.01%.
Report is 3 commits behind head on develop.
The diff coverage is 82.02%.

@@             Coverage Diff             @@
##           develop    #6712      +/-   ##
===========================================
- Coverage    78.69%   78.69%   -0.01%     
===========================================
  Files         1147     1149       +2     
  Lines       100321   100410      +89     
  Branches     11714    11723       +9     
===========================================
+ Hits         78951    79013      +62     
- Misses       19497    19511      +14     
- Partials      1873     1886      +13

Flag	Coverage Δ
backend	`80.11% <82.02%> (-0.03%)`	⬇️
frontend_unit	`76.15% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...lchen/chain/ethereum/modules/thegraph/constants.py	`100.00% <100.00%> (ø)`
...ehlchen/chain/ethereum/modules/thegraph/decoder.py	`81.39% <81.39%> (ø)`

... and 20 files with indirect coverage changes

LefterisJP

Left some comments

rotkehlchen/chain/ethereum/modules/the_graph/constants.py

rotkehlchen/chain/ethereum/modules/the_graph/decoder.py

rotkehlchen/tests/unit/decoders/test_the_graph.py

rotkehlchen/chain/ethereum/modules/the_graph/decoder.py

LefterisJP

Some smaller comments

LefterisJP · 2023-10-06T21:10:31Z

rotkehlchen/chain/ethereum/modules/thegraph/decoder.py

+
+                # also account for the GRT burnt due to delegation tax
+                tokens_burnt = initial_amount_norm - stake_amount_norm
+                if tokens_burnt > 0:


Shouldn't this always be true? Or just going for some kidn of future proof thing where maybe the protocol removes the fee?

Should always be true, yes, so it's just a sanity-check to not send a negative amount into a new EvmEvent instance.

rotkehlchen/chain/ethereum/modules/thegraph/decoder.py

rotkehlchen/tests/unit/decoders/test_main.py

rotkehlchen/tests/unit/decoders/test_thegraph.py

LefterisJP

Also the PR has 7 commits already. It should only have 1. Or 2. One for implementation and 1 for tests. As linked before in contribution guide we keep commits doing one thing and if multiple commits are for same reason combine them.

LefterisJP

Thanks. Looks good to me, will merge.

Do the small nitpicks in the next PR and keep them in mind. Also you would need to add a changelog in the next PR.

LefterisJP · 2023-10-09T10:46:02Z

rotkehlchen/chain/ethereum/modules/thegraph/decoder.py

+        delegator = hex_or_bytes_to_address(context.tx_log.topics[2])
+        if delegator is None or self.base.is_tracked(delegator) is False:
+            return DEFAULT_DECODING_OUTPUT
+        indexer = hex_or_bytes_to_address(context.tx_log.topics[1])


Use newline after if. Helps readability

Suggested change

indexer = hex_or_bytes_to_address(context.tx_log.topics[1])

indexer = hex_or_bytes_to_address(context.tx_log.topics[1])

LefterisJP · 2023-10-09T10:46:59Z

rotkehlchen/chain/ethereum/modules/thegraph/decoder.py

+        delegator = hex_or_bytes_to_address(context.tx_log.topics[2])
+        if delegator is None or self.base.is_tracked(delegator) is False:
+            return DEFAULT_DECODING_OUTPUT
+        indexer = hex_or_bytes_to_address(context.tx_log.topics[1])


Use newline after if. Helps readability

Suggested change

indexer = hex_or_bytes_to_address(context.tx_log.topics[1])

indexer = hex_or_bytes_to_address(context.tx_log.topics[1])

LefterisJP · 2023-10-09T10:47:17Z

rotkehlchen/chain/ethereum/modules/thegraph/decoder.py

+        delegator = hex_or_bytes_to_address(context.tx_log.topics[2])
+        if delegator is None or self.base.is_tracked(delegator) is False:
+            return DEFAULT_DECODING_OUTPUT
+        indexer = hex_or_bytes_to_address(context.tx_log.topics[1])


Use newline after if. Helps readability

Suggested change

indexer = hex_or_bytes_to_address(context.tx_log.topics[1])

indexer = hex_or_bytes_to_address(context.tx_log.topics[1])

LefterisJP · 2023-10-09T10:50:25Z

rotkehlchen/tests/unit/decoders/test_thegraph.py

+                'Withdraw 1003.70342593701668535 GRT'
+                ' from indexer 0x6125eA331851367716beE301ECDe7F38A7E429e7'


This is also a nitpick and can't enforce from linter but I have notices when you split a sentence string across multiple lines you end the line without a space and start the new with a space. Can you try to be doing the opposite? It's how we do it in the rest of the code.

Really minor nitpick to keep in mind. Here and in all other places you do it.

Suggested change

'Withdraw 1003.70342593701668535 GRT'

' from indexer 0x6125eA331851367716beE301ECDe7F38A7E429e7'

'Withdraw 1003.70342593701668535 GRT '

'from indexer 0x6125eA331851367716beE301ECDe7F38A7E429e7'

gitpoap-bot · 2023-10-09T10:51:19Z

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2023 rotki Contributor:

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

rotkibot · 2023-10-09T10:51:32Z

rotki/test-caching/tree/issue_2044_the_graph_staking was successfully merged

d347h-eth commented Oct 6, 2023

View reviewed changes

rotkehlchen/chain/ethereum/modules/the_graph/constants.py Outdated Show resolved Hide resolved

d347h-eth mentioned this pull request Oct 6, 2023

Flaky sqlite failure - Error binding parameter 0 - probably unsupported type #5432

Open

d347h-eth commented Oct 6, 2023

View reviewed changes

rotkehlchen/chain/ethereum/modules/the_graph/decoder.py Outdated Show resolved Hide resolved

d347h-eth added the ready for peer review Backend PR ready to be reviewed by colleagues label Oct 6, 2023

LefterisJP reviewed Oct 6, 2023

View reviewed changes

d347h-eth force-pushed the issue_2044_the_graph_staking branch from 3a5b592 to fd2972e Compare October 9, 2023 09:27

add decoder for The Graph's delegator staking

61099ca

d347h-eth force-pushed the issue_2044_the_graph_staking branch from fd2972e to 61099ca Compare October 9, 2023 09:28

LefterisJP approved these changes Oct 9, 2023

View reviewed changes

LefterisJP merged commit dec19d6 into rotki:develop Oct 9, 2023
15 checks passed

LefterisJP temporarily deployed to cassette-merge October 9, 2023 10:51 — with GitHub Actions Inactive

d347h-eth removed the ready for peer review Backend PR ready to be reviewed by colleagues label Oct 9, 2023

d347h-eth mentioned this pull request Oct 9, 2023

add accounting for The Graph staking #6720

Merged

LefterisJP mentioned this pull request Oct 13, 2023

Add GRT support for tracking delegation and delegation and indexing rewards. #2044

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Decoder for The Graph staking #6712

Add Decoder for The Graph staking #6712

d347h-eth commented Oct 6, 2023 •

edited

CLAassistant commented Oct 6, 2023 •

edited

codecov bot commented Oct 6, 2023 •

edited

LefterisJP left a comment

LefterisJP left a comment

LefterisJP Oct 6, 2023

d347h-eth Oct 9, 2023

LefterisJP left a comment

LefterisJP left a comment

LefterisJP Oct 9, 2023

LefterisJP Oct 9, 2023

LefterisJP Oct 9, 2023

LefterisJP Oct 9, 2023

gitpoap-bot bot commented Oct 9, 2023

rotkibot commented Oct 9, 2023

	indexer = hex_or_bytes_to_address(context.tx_log.topics[1])

	indexer = hex_or_bytes_to_address(context.tx_log.topics[1])

		'Withdraw 1003.70342593701668535 GRT'
		' from indexer 0x6125eA331851367716beE301ECDe7F38A7E429e7'

Add Decoder for The Graph staking #6712

Add Decoder for The Graph staking #6712

Conversation

d347h-eth commented Oct 6, 2023 • edited

CLAassistant commented Oct 6, 2023 • edited

codecov bot commented Oct 6, 2023 • edited

Codecov Report

LefterisJP left a comment

Choose a reason for hiding this comment

LefterisJP left a comment

Choose a reason for hiding this comment

LefterisJP Oct 6, 2023

Choose a reason for hiding this comment

d347h-eth Oct 9, 2023

Choose a reason for hiding this comment

LefterisJP left a comment

Choose a reason for hiding this comment

LefterisJP left a comment

Choose a reason for hiding this comment

LefterisJP Oct 9, 2023

Choose a reason for hiding this comment

LefterisJP Oct 9, 2023

Choose a reason for hiding this comment

LefterisJP Oct 9, 2023

Choose a reason for hiding this comment

LefterisJP Oct 9, 2023

Choose a reason for hiding this comment

gitpoap-bot bot commented Oct 9, 2023

rotkibot commented Oct 9, 2023

d347h-eth commented Oct 6, 2023 •

edited

CLAassistant commented Oct 6, 2023 •

edited

codecov bot commented Oct 6, 2023 •

edited