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: block properties behaviour #853

Merged
merged 2 commits into from
Jul 16, 2024
Merged

Conversation

natanasow
Copy link
Contributor

Description:

When retrieving in smart contracts, not all block properties match what's returned to hardhat.

  • Retrieving a blockhash through hardhat returns an actual hash value, but returning it from within a smart contract returns 0x0000000000000000000000000000000000000000000000000000000000000000.
  • Retrieving the block's coinbase returns a different value when called in the smart contract.

Steps to reproduce;

  • The test should be able to get the hash of a given block when the block number is one of the 256 most recent blocks in BlockInfo reproduces the hash value issue. - fixed ✅
  • The test should get the current block miners address using block.coinbase reproduces the coinbase issue. - not reproducible ❌

Related issue(s):

Fixes #436

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Signed-off-by: nikolay <[email protected]>
@@ -7,8 +7,8 @@ contract BlockInfo {
return block.basefee;
}

function getBlockHash() public view returns (bytes32) {
return blockhash(block.number - 1);
function getBlockHash(uint256 blockNumber) public view returns (bytes32) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added blockNumber param for consistency because block.number - 1 depends on the current block.

@@ -26,7 +26,7 @@ describe('@solidityequiv1 BlockInfo Test Suite', function () {

before(async function () {
signers = await ethers.getSigners();
provider = ethers.getDefaultProvider();
provider = signers[0].provider;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ethers.getDefaultProvider(); returns FallbackProvider that queries Ethereum Mainnet and that's why we get inconsistent results in the tests below. We should use the signer's provider which is a JsonRpcProvider connected to the relay (described in hardhat.config.js).

const blockNumber = await provider.getBlockNumber();
const block = await provider.getBlock(blockNumber);
const blockHash = await blockInfo.getBlockHash(blockNumber);
expect(block.hash).to.equal(blockHash);
});

it('should get the current block coinbase which is the hedera network account', async function () {
Copy link
Contributor Author

@natanasow natanasow Jul 15, 2024

Choose a reason for hiding this comment

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

@ebadiere the problem related to the coinbase address (described in the issue) is not reproducible, could you share a little more info on how to reproduce it or verify that it's already fixed in that PR?

Copy link
Contributor

@ebadiere ebadiere Jul 16, 2024

Choose a reason for hiding this comment

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

Agreed. The test passes. Not an issue now.

@natanasow natanasow self-assigned this Jul 15, 2024
@natanasow natanasow marked this pull request as ready for review July 15, 2024 09:13
Copy link

Test Results

  16 files  +  1    77 suites  +4   7m 29s ⏱️ +16s
254 tests +  6  247 ✔️ +  2  7 💤 +4  0 ±0 
263 runs  +15  255 ✔️ +10  8 💤 +5  0 ±0 

Results for commit 3cda21e. ± Comparison against base commit 96138c2.

Copy link
Contributor

@ebadiere ebadiere left a comment

Choose a reason for hiding this comment

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

LG.

@ebadiere ebadiere merged commit 19a0615 into main Jul 16, 2024
27 checks passed
@ebadiere ebadiere deleted the 436-block-properties-behaviour branch July 16, 2024 19:43
@quiet-node quiet-node added the enhancement New feature or request label Aug 12, 2024
@quiet-node quiet-node added this to the 0.10.0 milestone Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Block properties behavior
3 participants