blockIdentifier object converts string decimals into pending blocktag

[Eikix]

Describe the bug
A clear and concise description of what the bug is.

In the block class:

  private setIdentifier(__identifier: BlockIdentifier) {
    if (isString(__identifier)) {
      if (isHex(__identifier)) {
        this.hash = __identifier;
      } else if (validBlockTags.includes(__identifier as BlockTag)) {
        this.tag = __identifier;
      }
    } else if (isBigInt(__identifier)) {
      this.hash = toHex(__identifier);
    } else if (isNumber(__identifier)) {
      this.number = __identifier;
    } else {
      this.tag = BlockTag.pending;
    }
  }

But in the type declaration, we get:

/**
 * hex string and BN are detected as block hashes
 * decimal string and number are detected as block numbers
 * null appends nothing to the request url
 */
type BlockIdentifier = BlockNumber | BigNumberish;

To Reproduce
Steps to reproduce the behavior

Do a provider.call("some_method", {blockIdentifier: "1020230320")) -> it'll be converted to block pending

Additional resources:

Logs from our docker shows that we're trying to query a certain block number, but the request itself defaults to blocktag pending

9f51448426d8931694e51db5795e6","entry_point_selector":"0x1197c0cc3e1cbbe95556c0e2cfbfede9b831b095b4ab4f04f14ad7ff918a72f","calldata":[]},"block_id":"pending"}
2024-04-09 15:52:34  20: Contract not found: undefined
2024-04-09 15:52:34 ⚠️ Failed to get coinbase for block 0x0000000000000e0b - Error: RPC: starknet_call with params {"request":{"contract_address":"0x612fb5de32723a19b073b3aba348e48a3d9f51448426d8931694e51db5795e6","entry_point_selector":"0x15cb1934fc042b6b0bd8880dbf0571efa4896dcea2163e8f4797890445f6cc4","calldata":[]},"block_id":"pending"}
2024-04-09 15:52:34  20: Contract not found: undefined
2024-04-09 15:52:34 ⚠️ Failed to get base fee for block 0x0000000000000e0b - Error: RPC: starknet_call with params

Associated code:

  try {
    const response = (await KAKAROT.call("get_coinbase", [], {
      // ⚠️ StarknetJS: blockIdentifier is a block hash if value is BigInt or HexString, otherwise it's a block number.
      blockIdentifier: BigInt(blockNumber).toString(),
    })) as {
      coinbase: bigint;
    };
    coinbase = response.coinbase;
  } catch (error) {
    console.warn(
      `⚠️ Failed to get coinbase for block ${blockNumber} - Error: ${error.message}`,
    );
    coinbase = BigInt(0);
  }

[PhilippeR26]

Hello,
Have you tested with several nodes : pathfinder, juno, devnet-rs?

[ivpavici]

@Eikix because it's you, I wont close the issue... but I vowed to close incomplete issues!
Incomplete in a sense that the bug template is removed and we are missing the version of Starknet.js and the environment you are using - I'm assuming it's Katana based on TG messages

[ivpavici]

With the latest devnet-rs and starknet.js 6.6.6 i get this:

1. from your example
   provider.call("some_method", {blockIdentifier: "1020230320")) -> it'll be converted to block pending

My code:

  try {
    const resp1 = (await myTestContract.call("return_u512", [serializedU512], {
      blockIdentifier: "12333",
    })) as bigint;
  } catch (error: any) {
    console.warn(
      `⚠️ Failed to get coinbase for block  - Error: ${error.message}`
    );
  }

gives:

⚠️ Failed to get coinbase for block  - Error: RPC: starknet_call with params {
  "request": {
    "contract_address": "0x3ba0caa9ab147b118e1589cde19fcc49318a17c2814decf0cce8e7db6ebec22",
    "entry_point_selector": "0xbffc6a437c6d569cf809118ae879d8c29850d01d30ffbd358142e8ca9e2abd",
    "calldata": [
      "0x0",
      "0x11111111111111111111111111111111",
      "0x22222222222222222222222222222222",
      "0x33333333333333333333333333333333"
    ]
  },
  "block_id": null
}

        -32602: Invalid block ID: null: undefined

On the other hand this change:

blockIdentifier: 12333, // converted from string to number

gives:

⚠️ Failed to get coinbase for block  - Error: RPC: starknet_call with params {
  "request": {
    "contract_address": "0x4ce538bc1a8bdfad4f81c432c77ffa2c657415e5332ad0bfaa1a17eb636ebcb",
    "entry_point_selector": "0xbffc6a437c6d569cf809118ae879d8c29850d01d30ffbd358142e8ca9e2abd",
    "calldata": [
      "0x0",
      "0x11111111111111111111111111111111",
      "0x22222222222222222222222222222222",
      "0x33333333333333333333333333333333"
    ]
  },
  "block_id": {
    "block_number": 12333
  }
}

        -1: No state at block Number(12333); consider running with --state-archive-capacity full: undefined

[Eikix]

With the latest devnet-rs and starknet.js 6.6.6 i get this:

1. from your example
   provider.call("some_method", {blockIdentifier: "1020230320")) -> it'll be converted to block pending

My code:

  try {
    const resp1 = (await myTestContract.call("return_u512", [serializedU512], {
      blockIdentifier: "12333",
    })) as bigint;
  } catch (error: any) {
    console.warn(
      `⚠️ Failed to get coinbase for block  - Error: ${error.message}`
    );
  }

gives:

⚠️ Failed to get coinbase for block  - Error: RPC: starknet_call with params {
  "request": {
    "contract_address": "0x3ba0caa9ab147b118e1589cde19fcc49318a17c2814decf0cce8e7db6ebec22",
    "entry_point_selector": "0xbffc6a437c6d569cf809118ae879d8c29850d01d30ffbd358142e8ca9e2abd",
    "calldata": [
      "0x0",
      "0x11111111111111111111111111111111",
      "0x22222222222222222222222222222222",
      "0x33333333333333333333333333333333"
    ]
  },
  "block_id": null
}

        -32602: Invalid block ID: null: undefined

On the other hand this change:

blockIdentifier: 12333, // converted from string to number

gives:

⚠️ Failed to get coinbase for block  - Error: RPC: starknet_call with params {
  "request": {
    "contract_address": "0x4ce538bc1a8bdfad4f81c432c77ffa2c657415e5332ad0bfaa1a17eb636ebcb",
    "entry_point_selector": "0xbffc6a437c6d569cf809118ae879d8c29850d01d30ffbd358142e8ca9e2abd",
    "calldata": [
      "0x0",
      "0x11111111111111111111111111111111",
      "0x22222222222222222222222222222222",
      "0x33333333333333333333333333333333"
    ]
  },
  "block_id": {
    "block_number": 12333
  }
}

        -1: No state at block Number(12333); consider running with --state-archive-capacity full: undefined

Thanks for the callout,
I should've given the StarknetJS version: 5.24.3.
I didn't say which environment I was using because for me the bug is not related to the underlying Starknet client.

The block of code from setIdentifier behaves in a surprising way. Maybe the word 'bug' is strong, but it's unexpected behaviour. Both in 6.6 or 5.24.3 ->

If blockIdentifier is a string decimal it'll either get translated as null or as pending which is surprising to the end user.
Additionally, blockIdentifier has little to no documentation attached to it except a link to the type declaration.

The problem started because the doc comment associated with the type is misleading:

/**
 * hex string and BN are detected as block hashes
 * decimal string and number are detected as block numbers
 * null appends nothing to the request url
 */
export type BlockIdentifier = BlockNumber | BigNumberish;

[ivpavici]

OK, the block class code you pasted in the issue is from v6.6.6... but it shouldn't matter, just for us to be on the same page!

5.24.3 is old, but still latest official version... we will release v6 officially soon!

But as far as I can see the part of the docs is the same in the latest docs as in v5, and it's not formatted well, we can think of something here + better explanaition / example

[Eikix]

OK, the block class code you pasted in the issue is from v6.6.6... but it shouldn't matter, just for us to be on the same page!

5.24.3 is old, but still latest official version... we will release v6 officially soon!

But as far as I can see the part of the docs is the same in the latest docs as in v5, and it's not formatted well, we can think of something here + better explanaition / example

You're right! This is consistent with our results:

  private setIdentifier(__identifier: BlockIdentifier) {
    if (typeof __identifier === 'string' && isHex(__identifier)) {
      this.hash = __identifier;
    } else if (typeof __identifier === 'bigint') {
      this.hash = toHex(__identifier);
    } else if (typeof __identifier === 'number') {
      this.number = __identifier;
    } else if (
      typeof __identifier === 'string' &&
      validBlockTags.includes(__identifier as BlockTag)
    ) {
      this.tag = __identifier;
    } else {
      // default
      this.tag = BlockTag.pending;
    }
  }

this is from 5.24.3, so string decimals will yield "pending" blocktag

whereas in 6.6.6, it'll yield null

[ivpavici]

"decimal string and number are detected as block numbers" -> the bolded part seems not correct

we only have these tests, should be expanded and fixed cc @tabaktoni :

describe('new Block()', () => {
  test('Block identifier and queryIdentifier', () => {
    const blockA = new Block(0);
    expect(blockA.identifier).toMatchObject({ block_number: 0 });
    expect(blockA.queryIdentifier).toBe('blockNumber=0');

    const blockB = new Block('latest');
    expect(blockB.identifier).toBe('latest');
    expect(blockB.queryIdentifier).toBe('blockNumber=latest');

    const blockC = new Block('0x01');
    expect(blockC.identifier).toMatchObject({ block_hash: '0x01' });
    expect(blockC.queryIdentifier).toBe('blockHash=0x01');
  });
});

[tabaktoni]

Thank you for pointing this out.
The main issue here is misleading docs and the decimal strings bug.

The decimal string is fixed.
Null produce tag 'pending'

[Eikix]

Niice:)! Thanks for the fast response and involvement

[github-actions]

🎉 This issue has been resolved in version 7.0.0 🎉

The release is available on:

• npm package (@next dist-tag)
• GitHub release

Your semantic-release bot 📦🚀