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

Non-Staked nodes are in Neighborhood 0x00000000000... #269

Open
ldeffenb opened this issue Sep 9, 2024 · 3 comments
Open

Non-Staked nodes are in Neighborhood 0x00000000000... #269

ldeffenb opened this issue Sep 9, 2024 · 3 comments

Comments

@ldeffenb
Copy link

ldeffenb commented Sep 9, 2024

The latest Redistribution Contract in the Sepolia Testnet causes non-staked nodes to believe they are participating in neighborhood 0. This is due to the isParticipatingInUpcomingRound method using lastUpdatedBlockNumberOfAddress and overlayOfAddress from the Staking contract, both of which return 0 for non-staked nodes.

function isParticipatingInUpcomingRound(address _owner, uint8 _depth) public view returns (bool) {
if (currentPhaseReveal()) {
revert WrongPhase();
}
if (Stakes.lastUpdatedBlockNumberOfAddress(_owner) >= block.number - 2 * ROUND_LENGTH) {
revert MustStake2Rounds();
}
return inProximity(Stakes.overlayOfAddress(_owner), currentRoundAnchor(), _depth);
}

Consider the following contract query captures for my sepolia testnet node address 0x0e8a050ae87932b00a1acb01718654ac9fbb4c1e and overlay 0xa006d6ef64db7cb9a43681c69f06044851cc8b4a15f82da6d9194184b2347b53 with no stake in the current staking contracts.

curl http://192.168.10.38:53310/addresses | jq
{
  "overlay": "a006d6ef64db7cb9a43681c69f06044851cc8b4a15f82da6d9194184b2347b53",
  "underlay": [
    <snip>  ],
  "ethereum": "0x0e8a050ae87932b00a1acb01718654ac9fbb4c1e",
  "publicKey": "0366bf06a2fa3d98c4fc867cb4c9e176eb794a173e6aa0cf2559d2ebc38f79750b",
  "pssPublicKey": "038619438d1f97f03fe8e28149b9dba8721df592a72ab3a6b6284b43dff6d2d5e9"
}
curl http://192.168.10.38:53310/stake -s | jq
{
  "withdrawableStake": "0"
}

From the Redistribution contract:
image
image

From the Staking contract:
image
image

This cause all non-staked nodes to calculate a hash and I have no idea what happens if they actually commit, reveal, and possibly try to claim a win in the round where their actual neighborhood wasn't actually selected.

@ldeffenb
Copy link
Author

ldeffenb commented Sep 9, 2024

Here's a sample commit transaction that failed because the node was not staked.
https://sepolia.etherscan.io/tx/0xb68b97044cbe7d892bb784479bac14f86059a9d8d8ecc4d427732361b28c8f65
I suspect all of these failed transactions are caused by this same bug.

0x73cdbfc1a7c0645e84cbab8595301d0a09d662dca4bc3cf55835f08df3a3142b
0x95ca0f85a8881fc4a69c9d8e56798fdb01dcde0da3d2be7036542061c73f327c
0x3ab992b81090fc177305c38e3c53e44be121286ca57810f958132f393998bf0c
0xd1c72c78d0d5513868646f3e3a9b8ccdc7d2b9d6cd2b654d1b471062a5083d25
0xfe68bb7182f22432091f88dd5907fdd0c82a5d218d785f266194567117fb0d6f
0xfed2ea6bf9fb7a7ba41f8a52d6818e9f02a33ce73f6a7263dc2eac06f0042944
0xd690b6174ec1666de0d801a793c1473831f54c0e86584262f01c4390b087179f 
0x456d20f76241e3d01f83197335a08f03c71576c13179b0a1ca7642eacd7cf776
0xb2b9160296f1920c41d73b29e3b1b91fa38ff1574671d80295dc9b559f032645
0x37e459869d5d0dfc2b4ef5ed6dcc94d0edaf1d57694e27c40c2e938ea18a783e
0x28e6155c698ca242fbd0d6639dcbd65116a23b6802c70ea251383961908fee5f
0x95991930a4769dbb1d14297c35ca75fcfc4bf7660fdc32721db7118ad29258a4
0xb01c855c663ff9dd813d1b2d606f61a9c5e84599ac643d7757573302a9c3fa4b
0x5967465189cd5bc2af2cf1303d448ec0eb0482aae8405f041caaf3ab0e4057af
0x943e66c8cdbaf0f7df5b73e5d11daa9cb69fb1300ea4efefefe68fa744080559
0xb68b97044cbe7d892bb784479bac14f86059a9d8d8ecc4d427732361b28c8f65

@ldeffenb
Copy link
Author

ldeffenb commented Sep 9, 2024

Here are the relevant lines from the commit transaction that causes the inappropriately generated hash to finally fail for non-staked nodes. A similar check is needed in isParticipatingInUpcomingRound instead of just running with the zeros.

uint256 _lastUpdate = Stakes.lastUpdatedBlockNumberOfAddress(msg.sender);

if (_lastUpdate == 0) {
revert NotStaked();
}

But isParticipatingInUpcomingRound should simply return false instead of an error, IMHO.

@0xCardinalError
Copy link
Contributor

yes, you are right, we should be including this kind of check in "isParticipatingInUpcomingRound".
Adding this to release, thank you

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

No branches or pull requests

2 participants