Skip to content

Fix proxy extranonce range overflow after adding additional_coinbase_data#146

Open
Priceless-P wants to merge 1 commit into
dmnd-pool:masterfrom
Priceless-P:fix/extranonce
Open

Fix proxy extranonce range overflow after adding additional_coinbase_data#146
Priceless-P wants to merge 1 commit into
dmnd-pool:masterfrom
Priceless-P:fix/extranonce

Conversation

@Priceless-P
Copy link
Copy Markdown
Collaborator

Following this change , m.extranonce_size grew from 16 to 32 bytes. Proxy currently double-counted the prefix when calculating extranonce ranges, this now causes invalidExtranonceRange on channel creation.

This PR corrects the math by treating tproxy_e1_len as the actual end of range_1.

Currently we added the prefix twice:

let range_1 = prefix_len..prefix_len + tproxy_e1_len; // downstream extranonce1
let range_2 = prefix_len + tproxy_e1_len
    ..prefix_len + m.extranonce_size as usize;

With this change, we use tproxy_e1_len directly:

let range_1 = prefix_len..tproxy_e1_len // 16..27
let  range_2 = tproxy_e1_len..extranonce_size // 27..32

Also changed extranonce2_len in bridge.rs:
let extranonce2_len = m.extranonce_size - prefix_len

Now 

it works but I’m not sure if it is working the way we want .

Fixes https://github.com/demand-open-source/demand-pool/issues/264

Looking forward to your review @Fi3 @jbesraa

let range_1 = prefix_len..prefix_len + tproxy_e1_len; // downstream extranonce1
let range_2 = prefix_len + tproxy_e1_len
..prefix_len + m.extranonce_size as usize; // extranonce2
let range_1 = prefix_len..tproxy_e1_len; // downstream extranonce1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

since you are at it remove the proxy_extranonce1_len function that looks reduntant

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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