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

Feat: Abstract contract interaction #432

Merged
merged 21 commits into from
May 6, 2024

Conversation

F4ever
Copy link
Member

@F4ever F4ever commented Apr 18, 2024

No description provided.

@F4ever F4ever changed the title Feat/abstract contract interaction Feat: Abstract contract interaction Apr 18, 2024

@lru_cache(maxsize=1)
def normalized_cl_reward_per_epoch(self, block_identifier: BlockIdentifier = 'latest') -> int:
response = self.functions.get('NORMALIZED_CL_REWARD_PER_EPOCH').call(block_identifier=block_identifier)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why other syntax?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is one function get that receives value by key.
To correctly work with cache and do not increase it's size, I think it's better to use different functions

Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't get it, to be honest.

Copy link
Contributor

Choose a reason for hiding this comment

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

@madlabman function get is the contract function that returns value by key. NORMALIZED_CL_REWARD_PER_EPOCH isn't state variable

Copy link
Contributor

Choose a reason for hiding this comment

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

@madlabman function get is the contract function that returns value by key. NORMALIZED_CL_REWARD_PER_EPOCH isn't state variable

Got it!

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@vgorkavenko vgorkavenko self-requested a review April 22, 2024 06:19
@@ -301,7 +277,7 @@ def _process_report_hash(self, blockstamp: ReferenceBlockStamp, report_hash: Hex
self._send_report_hash(blockstamp, report_hash, self.CONSENSUS_VERSION)
return None

def _process_report_data(self, blockstamp: ReferenceBlockStamp, report_data: tuple, report_hash: HexBytes):
def _process_report_data(self, blockstamp: ReferenceBlockStamp, report_data: tuple, report_hash: bytes):
Copy link
Contributor

Choose a reason for hiding this comment

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

can't found the related changes in place where it is called. is it safe?

Copy link
Contributor

Choose a reason for hiding this comment

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

HexBytes just rewrites a few methods related to a representation.

Copy link
Member Author

Choose a reason for hiding this comment

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

w3.keccak() returns HexBytes, but in annotation indicated bytes.
Returned back



class AccountingOracleContract(BaseOracleContract):
abi_path = './assets/AccountingOracle.json'
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay with the current, but it will be more obvious if you use cwd somewhere


@lru_cache(maxsize=1)
def normalized_cl_reward_per_epoch(self, block_identifier: BlockIdentifier = 'latest') -> int:
response = self.functions.get('NORMALIZED_CL_REWARD_PER_EPOCH').call(block_identifier=block_identifier)
Copy link
Contributor

Choose a reason for hiding this comment

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

@madlabman function get is the contract function that returns value by key. NORMALIZED_CL_REWARD_PER_EPOCH isn't state variable


response = Web3.to_int(response)
logger.info({
'msg': 'Call `NORMALIZED_CL_REWARD_PER_EPOCH()`.',
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not true. We call get function with NORMALIZED_CL_REWARD_PER_EPOCH arg

Copy link
Contributor

Choose a reason for hiding this comment

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

let's make _get function and reuse it

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that's why I didn't get the get call :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thx!
Fixed

Comment on lines +16 to +17
@lru_cache(maxsize=1)
@list_of_dataclasses(StakingModule)
Copy link
Contributor

Choose a reason for hiding this comment

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

here we use @list_of_dataclasses but not for get_all_node_operator_digests and get_all_node_operator_digests func. let's use a common approach

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if this is easy to do. To create NodeOperator we have to provide a module that is not in the response. That's why I'm doing parsing inside method

Copy link
Contributor

@vgorkavenko vgorkavenko Apr 24, 2024

Choose a reason for hiding this comment

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

That should work, I guess (regarding to test_dataclasses_utils)

    @lru_cache(maxsize=1)
    @list_of_dataclasses(NodeOperator)
    def get_all_node_operator_digests(self, module: StakingModule, block_identifier: BlockIdentifier = 'latest') -> list[NodeOperator]:
        """
        Returns node operator digest for each node operator in lido protocol
        """
        response = self.functions.getAllNodeOperatorDigests(module.id).call(block_identifier=block_identifier)
        response = [{**no, "staking_module": module} for no in response]

        logger.info({
            'msg': f'Call `getAllNodeOperatorDigests({module.id})`.',
            'value': response,
            'block_identifier': repr(block_identifier),
        })
        return response

Copy link
Member Author

Choose a reason for hiding this comment

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

In this example, you run twice over all NOs.

  1. Regenerate all dicts with staking module
  2. From dicts to dataclasses

Copy link
Member Author

Choose a reason for hiding this comment

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

But it already doesn't matter. I've rewritten this function in next PR

@F4ever F4ever merged commit 2af069b into feat/oracle-v4 May 6, 2024
6 checks passed
@F4ever F4ever mentioned this pull request May 6, 2024
@F4ever F4ever mentioned this pull request Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants