pokt-network / pocket-core

Official implementation of the Pocket Network Protocol
http://www.pokt.network
MIT License
208 stars 101 forks source link

[v0.11 feature] Built-in Reward Share #1581

Closed msmania closed 7 months ago

msmania commented 10 months ago

Description

This patch implements the Built-in Reward Share feature which lets the network itself to distribute the relay/block rewards into multiple addresses. The change consists of the following parts.

This change is consensus-breaking. The new behavior is put behind a new feature key RewardDelegator. The new field is kept unavailable until activation.

The new structure of Validator or MsgStake is backward/forward compatible, meaning the new binary can still unmarshal data marshaled by an older binary, and vice versa. In other words, the network before RewardDelegator activation accepts an MsgStake transaction, ignoring the RewardDelegators field. And the new Validator structure can handle all historical states from genesis. Therefore this patch does not introduce a structure like 10.0Validaor as the NCUST patch did before.

Summary generated by Reviewpad on 26 Dec 23 01:21 UTC

This pull request includes changes to multiple files. Here is a summary of the diff:

  1. The file common_test.go was modified to replace the import of math/rand with crypto/rand. Additionally, the comment // : deadcode unused was removed.

  2. The file x/nodes/keeper/abci_test.go was modified to add and remove import statements, as well as comment out unnecessary code related to state conversion.

  3. The file x/nodes/types/validator.go was modified to add an import, add a new field to the Validator struct, add a new function to create a validator from a message, modify several methods to include a new field in the output, and add a new struct and comment.

  4. The file x/nodes/types/validator_test.go was modified to add import statements and a new test function.

  5. The file msg_test.go was modified to add and remove import statements, add new test functions, and update existing test functions.

  6. The file keeper_test.go was modified to add import statements, modify existing test functions, and add new test functions.

  7. The file go.mod was modified to add and update package requirements.

  8. The file handler.go was modified to add import statements and modify function implementations.

  9. The file nodes.proto was modified to remove an import statement and add a new field to a message.

  10. The file msg.go was modified to add import statements, add a new struct and function, and modify existing methods.

  11. The file genesis_test.go was modified to add import statements and modify existing test functions.

  12. The file rpc_test.go was modified to add and remove import statements, modify function implementations, and add test cases.

  13. The file expectedKeepers.go was modified to remove comments and add a new method.

  14. The file config.go was modified to add a new field to a struct.

  15. The file msg.proto was modified to add a new field to a message.

  16. The file LegacyValidator.go was modified to add a new method and update existing methods.

  17. The file errors.go was modified to add new error codes and functions to handle them.

  18. The file reward_test.go was modified to add import statements, add and update test functions.

  19. The file util_test.go was modified to rearrange import statements and add new test functions.

Please review these changes and provide any necessary feedback. Let me know if you need more information or if there's anything else I can assist you with.

POKT-Discourse commented 10 months ago

This pull request has been mentioned on Pocket Network Forum. There might be relevant details there:

https://forum.pokt.network/t/unleashing-the-potential-of-pocket/4720/1

POKT-Discourse commented 9 months ago

This pull request has been mentioned on Pocket Network Forum. There might be relevant details there:

https://forum.pokt.network/t/unleashing-the-potential-of-non-custodial-node-running/4796/1

Olshansk commented 7 months ago

@msmania Please note that I updated the base to tokikuch/RTTM2 so its easier to review. We HAVE TO make sure to keep it up to date with that branch and MAKE SURE to update it back to main before meging.

msmania commented 7 months ago

@msmania I took another look and I think this is as good of a review as I can do too (reaching PR exhaustion).

I added one small NIT, one #PUC request, but otherwise the only blocker is to get the CI green after we merge the other PR.

Thank you! I created https://github.com/pokt-network/pocket-core/pull/1589 to get the CI green.

msmania commented 7 months ago

@msmania I took another look and I think this is as good of a review as I can do too (reaching PR exhaustion).

I added one small NIT, one #PUC request, but otherwise the only blocker is to get the CI green after we merge the other PR.

Comparing the test results of #1589 to the results of #1587, the following tests failed in both of the test runs.

The difference is TestRPC_QueryUnconfirmedTxs, which I think is flaky and can be ignored as it failed on #1582 too as mentioned here. So I think this is good to be merged. What do you think, @Olshansk?

msmania commented 7 months ago

1589 got a green light.

msmania commented 7 months ago

@Olshansk Looks like a force-push revoked your approval. Can you sign off again?

POKT-Discourse commented 5 months ago

This pull request has been mentioned on Pocket Network Forum. There might be relevant details there:

https://forum.pokt.network/t/rc-0-11-1-upgrade-and-hi/5012/1