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

Add basic Polkadot support #4

Closed
wants to merge 6 commits into from
Closed

Add basic Polkadot support #4

wants to merge 6 commits into from

Conversation

Har01d
Copy link
Collaborator

@Har01d Har01d commented Apr 26, 2023

This is still somewhat WIP as the proposed module only supports some basic transfers.

Copy link
Collaborator

@alexqrid alexqrid left a comment

Choose a reason for hiding this comment

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

There is a big problem with misunderstanding uniqueness of the hash, for more info look through https://wiki.polkadot.network/docs/build-protocol-info#unique-identifiers-for-extrinsics f

$this->module = 'polkadot-minimal';
$this->is_main = true;
$this->currency = 'polkadot';
$this->currency_details = ['name' => 'DOT', 'symbol' => 'DOT', 'decimals' => 10, 'description' => null];
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is better to keep a comment about redenomination on block 1 248 328


/* This module processes some basic Polkadot transfers. It requires `subscan-explorer/subscan-essentials` to operate,
* see https://github.com/subscan-explorer/subscan-essentials/blob/master/docs/index.md for details. Please note that
* Polkadot has extrinsics instead of transactions, and not of them have "transaction hashes". Generally, this module
Copy link
Collaborator

Choose a reason for hiding this comment

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

fix typo pls: "... not all of them..."

$multi_curl = [];

foreach ($extrinsics_with_hashes as $extrinsic_hash)
$multi_curl[] = requester_multi_prepare($this->select_node(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, this is the wrong way, as the hash depends on extrisics body, i.e. there can be many extrinsics with the same hash.
Why don't you use block_id-extrinsic_id,i.e. extrinsic_index.
Also subscan API returns a single extrinsic for the specified hash, returning the array of extrinsics for the hash was one of our customization.

Anyway using current strategy with hash leads to reprocess extrinsics, that were already processed.
Use extrinsic_index instead of hash, as it's a unique key

if ((int)$k['block_num'] === $block_id)
{
if ($found)
throw new ModuleError('Two extrinsics with the same hash in one block');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not an error, this is a documented behaviour for polkadot, there's no need for the logging this as error.

foreach ($transfers as $transfer)
{
$events[] = [
'transaction' => $transfer['hash'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

again, this is not a unique identifier, use block_number-extrinsic_number instead, it can also be interpreted as a string.

@alexqrid
Copy link
Collaborator

closing in favour of #68

@alexqrid alexqrid closed this Feb 27, 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.

2 participants