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.

Sign up for GitHub

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

Jump to bottom

Improve isValidAddress validation #4440

Merged
merged 1 commit into from Aug 19, 2022
Merged

Improve isValidAddress validation #4440

merged 1 commit into from Aug 19, 2022

Conversation

dapplion
Copy link
Contributor

Motivation

Properly validate hex address

Description

Improve isValidAddress validation

@dapplion dapplion requested a review from a team as a code owner August 18, 2022 13:46
@github-actions
Copy link
Contributor

Performance Report

✔️ no performance regression detected

Full benchmark results
Benchmark suite Current: ef2d13e Previous: 9ffdbfa Ratio
getPubkeys - index2pubkey - req 1000 vs - 250000 vc 2.0233 ms/op 2.4179 ms/op 0.84
getPubkeys - validatorsArr - req 1000 vs - 250000 vc 65.376 us/op 87.477 us/op 0.75
BLS verify - blst-native 2.1658 ms/op 2.6841 ms/op 0.81
BLS verifyMultipleSignatures 3 - blst-native 4.4725 ms/op 5.7157 ms/op 0.78
BLS verifyMultipleSignatures 8 - blst-native 9.6773 ms/op 11.658 ms/op 0.83
BLS verifyMultipleSignatures 32 - blst-native 35.187 ms/op 43.537 ms/op 0.81
BLS aggregatePubkeys 32 - blst-native 46.641 us/op 58.647 us/op 0.80
BLS aggregatePubkeys 128 - blst-native 182.70 us/op 228.15 us/op 0.80
getAttestationsForBlock 159.42 ms/op 198.66 ms/op 0.80
isKnown best case - 1 super set check 478.00 ns/op 509.00 ns/op 0.94
isKnown normal case - 2 super set checks 466.00 ns/op 485.00 ns/op 0.96
isKnown worse case - 16 super set checks 465.00 ns/op 493.00 ns/op 0.94
CheckpointStateCache - add get delete 9.1590 us/op 10.967 us/op 0.84
validate gossip signedAggregateAndProof - struct 5.0220 ms/op 6.3082 ms/op 0.80
validate gossip attestation - struct 2.3678 ms/op 3.0692 ms/op 0.77
altair verifyImport mainnet_s3766816:31 4.8576 s/op 5.8872 s/op 0.83
pickEth1Vote - no votes 2.0503 ms/op 2.4676 ms/op 0.83
pickEth1Vote - max votes 18.338 ms/op 25.442 ms/op 0.72
pickEth1Vote - Eth1Data hashTreeRoot value x2048 12.380 ms/op 18.061 ms/op 0.69
pickEth1Vote - Eth1Data hashTreeRoot tree x2048 22.048 ms/op 24.884 ms/op 0.89
pickEth1Vote - Eth1Data fastSerialize value x2048 1.4774 ms/op 1.7626 ms/op 0.84
pickEth1Vote - Eth1Data fastSerialize tree x2048 10.978 ms/op 14.945 ms/op 0.73
bytes32 toHexString 880.00 ns/op 1.1770 us/op 0.75
bytes32 Buffer.toString(hex) 743.00 ns/op 829.00 ns/op 0.90
bytes32 Buffer.toString(hex) from Uint8Array 974.00 ns/op 1.1130 us/op 0.88
bytes32 Buffer.toString(hex) + 0x 758.00 ns/op 842.00 ns/op 0.90
Object access 1 prop 0.31900 ns/op 0.42000 ns/op 0.76
Map access 1 prop 0.30000 ns/op 0.34700 ns/op 0.86
Object get x1000 11.203 ns/op 16.300 ns/op 0.69
Map get x1000 0.97400 ns/op 0.96100 ns/op 1.01
Object set x1000 64.973 ns/op 110.68 ns/op 0.59
Map set x1000 44.533 ns/op 79.549 ns/op 0.56
Return object 10000 times 0.43580 ns/op 0.41770 ns/op 1.04
Throw Error 10000 times 5.9235 us/op 7.0861 us/op 0.84
enrSubnets - fastDeserialize 64 bits 2.4770 us/op 3.0500 us/op 0.81
enrSubnets - ssz BitVector 64 bits 772.00 ns/op 851.00 ns/op 0.91
enrSubnets - fastDeserialize 4 bits 389.00 ns/op 414.00 ns/op 0.94
enrSubnets - ssz BitVector 4 bits 746.00 ns/op 805.00 ns/op 0.93
prioritizePeers score -10:0 att 32-0.1 sync 2-0 77.834 us/op 110.11 us/op 0.71
prioritizePeers score 0:0 att 32-0.25 sync 2-0.25 122.78 us/op 150.79 us/op 0.81
prioritizePeers score 0:0 att 32-0.5 sync 2-0.5 193.51 us/op 267.26 us/op 0.72
prioritizePeers score 0:0 att 64-0.75 sync 4-0.75 336.05 us/op 595.53 us/op 0.56
prioritizePeers score 0:0 att 64-1 sync 4-1 404.56 us/op 616.55 us/op 0.66
RateTracker 1000000 limit, 1 obj count per request 176.44 ns/op 215.60 ns/op 0.82
RateTracker 1000000 limit, 2 obj count per request 129.00 ns/op 156.72 ns/op 0.82
RateTracker 1000000 limit, 4 obj count per request 104.92 ns/op 124.74 ns/op 0.84
RateTracker 1000000 limit, 8 obj count per request 92.263 ns/op 116.21 ns/op 0.79
RateTracker with prune 3.5900 us/op 4.9340 us/op 0.73
array of 16000 items push then shift 51.567 us/op 5.3437 us/op 9.65
LinkedList of 16000 items push then shift 14.643 ns/op 27.548 ns/op 0.53
array of 16000 items push then pop 180.95 ns/op 238.41 ns/op 0.76
LinkedList of 16000 items push then pop 13.890 ns/op 23.173 ns/op 0.60
array of 24000 items push then shift 77.330 us/op 8.5070 us/op 9.09
LinkedList of 24000 items push then shift 16.544 ns/op 27.000 ns/op 0.61
array of 24000 items push then pop 186.80 ns/op 245.22 ns/op 0.76
LinkedList of 24000 items push then pop 14.301 ns/op 23.171 ns/op 0.62
intersect bitArray bitLen 8 10.652 ns/op 12.434 ns/op 0.86
intersect array and set length 8 127.40 ns/op 185.30 ns/op 0.69
intersect bitArray bitLen 128 58.903 ns/op 72.050 ns/op 0.82
intersect array and set length 128 1.6909 us/op 2.3241 us/op 0.73
Buffer.concat 32 items 1.7260 ns/op 2.2500 ns/op 0.77
pass gossip attestations to forkchoice per slot 2.8593 ms/op 3.5542 ms/op 0.80
computeDeltas 3.5678 ms/op 4.6458 ms/op 0.77
computeProposerBoostScoreFromBalances 804.03 us/op 911.51 us/op 0.88
altair processAttestation - 250000 vs - 7PWei normalcase 3.6002 ms/op 5.1772 ms/op 0.70
altair processAttestation - 250000 vs - 7PWei worstcase 6.0749 ms/op 7.2496 ms/op 0.84
altair processAttestation - setStatus - 1/6 committees join 170.80 us/op 250.75 us/op 0.68
altair processAttestation - setStatus - 1/3 committees join 334.98 us/op 464.37 us/op 0.72
altair processAttestation - setStatus - 1/2 committees join 481.13 us/op 689.55 us/op 0.70
altair processAttestation - setStatus - 2/3 committees join 635.50 us/op 862.84 us/op 0.74
altair processAttestation - setStatus - 4/5 committees join 885.66 us/op 1.2213 ms/op 0.73
altair processAttestation - setStatus - 100% committees join 1.0752 ms/op 1.5146 ms/op 0.71
altair processBlock - 250000 vs - 7PWei normalcase 23.923 ms/op 33.641 ms/op 0.71
altair processBlock - 250000 vs - 7PWei normalcase hashState 38.626 ms/op 56.209 ms/op 0.69
altair processBlock - 250000 vs - 7PWei worstcase 86.351 ms/op 115.03 ms/op 0.75
altair processBlock - 250000 vs - 7PWei worstcase hashState 116.05 ms/op 120.96 ms/op 0.96
phase0 processBlock - 250000 vs - 7PWei normalcase 3.5928 ms/op 3.9766 ms/op 0.90
phase0 processBlock - 250000 vs - 7PWei worstcase 51.885 ms/op 63.966 ms/op 0.81
altair processEth1Data - 250000 vs - 7PWei normalcase 1.1354 ms/op 1.4840 ms/op 0.77
Tree 40 250000 create 690.02 ms/op 1.0083 s/op 0.68
Tree 40 250000 get(125000) 248.47 ns/op 326.94 ns/op 0.76
Tree 40 250000 set(125000) 1.9165 us/op 3.0655 us/op 0.63
Tree 40 250000 toArray() 27.550 ms/op 38.649 ms/op 0.71
Tree 40 250000 iterate all - toArray() + loop 28.720 ms/op 39.040 ms/op 0.74
Tree 40 250000 iterate all - get(i) 117.26 ms/op 132.57 ms/op 0.88
MutableVector 250000 create 19.647 ms/op 25.682 ms/op 0.77
MutableVector 250000 get(125000) 10.635 ns/op 13.866 ns/op 0.77
MutableVector 250000 set(125000) 419.13 ns/op 769.61 ns/op 0.54
MutableVector 250000 toArray() 5.5162 ms/op 7.1351 ms/op 0.77
MutableVector 250000 iterate all - toArray() + loop 5.8549 ms/op 8.0038 ms/op 0.73
MutableVector 250000 iterate all - get(i) 2.6820 ms/op 3.5144 ms/op 0.76
Array 250000 create 5.5784 ms/op 6.5985 ms/op 0.85
Array 250000 clone - spread 2.3932 ms/op 2.9943 ms/op 0.80
Array 250000 get(125000) 1.1470 ns/op 1.2390 ns/op 0.93
Array 250000 set(125000) 1.1480 ns/op 1.4050 ns/op 0.82
Array 250000 iterate all - loop 151.58 us/op 146.11 us/op 1.04
effectiveBalanceIncrements clone Uint8Array 300000 36.560 us/op 95.038 us/op 0.38
effectiveBalanceIncrements clone MutableVector 300000 707.00 ns/op 935.00 ns/op 0.76
effectiveBalanceIncrements rw all Uint8Array 300000 249.02 us/op 300.31 us/op 0.83
effectiveBalanceIncrements rw all MutableVector 300000 137.31 ms/op 246.46 ms/op 0.56
phase0 afterProcessEpoch - 250000 vs - 7PWei 185.69 ms/op 207.62 ms/op 0.89
phase0 beforeProcessEpoch - 250000 vs - 7PWei 112.70 ms/op 157.41 ms/op 0.72
altair processEpoch - mainnet_e81889 642.81 ms/op 802.84 ms/op 0.80
mainnet_e81889 - altair beforeProcessEpoch 180.63 ms/op 239.93 ms/op 0.75
mainnet_e81889 - altair processJustificationAndFinalization 16.843 us/op 65.240 us/op 0.26
mainnet_e81889 - altair processInactivityUpdates 8.4013 ms/op 11.150 ms/op 0.75
mainnet_e81889 - altair processRewardsAndPenalties 154.97 ms/op 206.68 ms/op 0.75
mainnet_e81889 - altair processRegistryUpdates 2.6250 us/op 14.581 us/op 0.18
mainnet_e81889 - altair processSlashings 625.00 ns/op 4.1460 us/op 0.15
mainnet_e81889 - altair processEth1DataReset 628.00 ns/op 4.2110 us/op 0.15
mainnet_e81889 - altair processEffectiveBalanceUpdates 2.3398 ms/op 3.0139 ms/op 0.78
mainnet_e81889 - altair processSlashingsReset 4.2480 us/op 28.577 us/op 0.15
mainnet_e81889 - altair processRandaoMixesReset 4.2120 us/op 25.112 us/op 0.17
mainnet_e81889 - altair processHistoricalRootsUpdate 698.00 ns/op 5.0750 us/op 0.14
mainnet_e81889 - altair processParticipationFlagUpdates 2.7100 us/op 16.758 us/op 0.16
mainnet_e81889 - altair processSyncCommitteeUpdates 613.00 ns/op 3.9390 us/op 0.16
mainnet_e81889 - altair afterProcessEpoch 194.55 ms/op 209.98 ms/op 0.93
phase0 processEpoch - mainnet_e58758 611.97 ms/op 811.71 ms/op 0.75
mainnet_e58758 - phase0 beforeProcessEpoch 228.78 ms/op 362.05 ms/op 0.63
mainnet_e58758 - phase0 processJustificationAndFinalization 15.311 us/op 61.554 us/op 0.25
mainnet_e58758 - phase0 processRewardsAndPenalties 132.22 ms/op 175.13 ms/op 0.76
mainnet_e58758 - phase0 processRegistryUpdates 7.8520 us/op 33.529 us/op 0.23
mainnet_e58758 - phase0 processSlashings 814.00 ns/op 3.2280 us/op 0.25
mainnet_e58758 - phase0 processEth1DataReset 677.00 ns/op 4.3490 us/op 0.16
mainnet_e58758 - phase0 processEffectiveBalanceUpdates 1.8339 ms/op 2.1046 ms/op 0.87
mainnet_e58758 - phase0 processSlashingsReset 3.7210 us/op 17.332 us/op 0.21
mainnet_e58758 - phase0 processRandaoMixesReset 4.1740 us/op 23.281 us/op 0.18
mainnet_e58758 - phase0 processHistoricalRootsUpdate 800.00 ns/op 4.5610 us/op 0.18
mainnet_e58758 - phase0 processParticipationRecordUpdates 4.4030 us/op 21.540 us/op 0.20
mainnet_e58758 - phase0 afterProcessEpoch 160.71 ms/op 177.34 ms/op 0.91
phase0 processEffectiveBalanceUpdates - 250000 normalcase 1.9944 ms/op 2.4781 ms/op 0.80
phase0 processEffectiveBalanceUpdates - 250000 worstcase 0.5 2.2600 ms/op 2.9279 ms/op 0.77
altair processInactivityUpdates - 250000 normalcase 51.357 ms/op 79.968 ms/op 0.64
altair processInactivityUpdates - 250000 worstcase 50.695 ms/op 55.086 ms/op 0.92
phase0 processRegistryUpdates - 250000 normalcase 5.9270 us/op 27.207 us/op 0.22
phase0 processRegistryUpdates - 250000 badcase_full_deposits 363.76 us/op 511.04 us/op 0.71
phase0 processRegistryUpdates - 250000 worstcase 0.5 226.18 ms/op 319.14 ms/op 0.71
altair processRewardsAndPenalties - 250000 normalcase 129.81 ms/op 175.28 ms/op 0.74
altair processRewardsAndPenalties - 250000 worstcase 135.20 ms/op 182.12 ms/op 0.74
phase0 getAttestationDeltas - 250000 normalcase 11.480 ms/op 14.835 ms/op 0.77
phase0 getAttestationDeltas - 250000 worstcase 11.828 ms/op 14.783 ms/op 0.80
phase0 processSlashings - 250000 worstcase 5.1475 ms/op 7.6117 ms/op 0.68
altair processSyncCommitteeUpdates - 250000 296.64 ms/op 341.25 ms/op 0.87
BeaconState.hashTreeRoot - No change 536.00 ns/op 563.00 ns/op 0.95
BeaconState.hashTreeRoot - 1 full validator 71.328 us/op 79.688 us/op 0.90
BeaconState.hashTreeRoot - 32 full validator 798.10 us/op 1.0320 ms/op 0.77
BeaconState.hashTreeRoot - 512 full validator 6.8113 ms/op 10.554 ms/op 0.65
BeaconState.hashTreeRoot - 1 validator.effectiveBalance 90.083 us/op 110.27 us/op 0.82
BeaconState.hashTreeRoot - 32 validator.effectiveBalance 1.4411 ms/op 1.7954 ms/op 0.80
BeaconState.hashTreeRoot - 512 validator.effectiveBalance 16.586 ms/op 20.074 ms/op 0.83
BeaconState.hashTreeRoot - 1 balances 63.105 us/op 84.051 us/op 0.75
BeaconState.hashTreeRoot - 32 balances 740.79 us/op 819.32 us/op 0.90
BeaconState.hashTreeRoot - 512 balances 6.1428 ms/op 7.3465 ms/op 0.84
BeaconState.hashTreeRoot - 250000 balances 108.87 ms/op 124.10 ms/op 0.88
aggregationBits - 2048 els - zipIndexesInBitList 24.402 us/op 33.852 us/op 0.72
regular array get 100000 times 61.627 us/op 59.185 us/op 1.04
wrappedArray get 100000 times 60.565 us/op 59.292 us/op 1.02
arrayWithProxy get 100000 times 29.029 ms/op 40.618 ms/op 0.71
ssz.Root.equals 483.00 ns/op 648.00 ns/op 0.75
byteArrayEquals 479.00 ns/op 510.00 ns/op 0.94
shuffle list - 16384 els 11.325 ms/op 12.206 ms/op 0.93
shuffle list - 250000 els 164.15 ms/op 178.53 ms/op 0.92
processSlot - 1 slots 12.608 us/op 16.677 us/op 0.76
processSlot - 32 slots 1.9240 ms/op 2.8218 ms/op 0.68
getEffectiveBalanceIncrementsZeroInactive - 250000 vs - 7PWei 362.16 us/op 704.30 us/op 0.51
getCommitteeAssignments - req 1 vs - 250000 vc 5.4752 ms/op 5.3994 ms/op 1.01
getCommitteeAssignments - req 100 vs - 250000 vc 8.0107 ms/op 7.9159 ms/op 1.01
getCommitteeAssignments - req 1000 vs - 250000 vc 8.6134 ms/op 8.5652 ms/op 1.01
RootCache.getBlockRootAtSlot - 250000 vs - 7PWei 7.8800 ns/op 11.390 ns/op 0.69
state getBlockRootAtSlot - 250000 vs - 7PWei 942.79 ns/op 1.2239 us/op 0.77
computeProposers - vc 250000 17.242 ms/op 21.204 ms/op 0.81
computeEpochShuffling - vc 250000 169.40 ms/op 184.97 ms/op 0.92
getNextSyncCommittee - vc 250000 288.97 ms/op 333.39 ms/op 0.87

by benchmarkbot/action

wemeetagain
wemeetagain reviewed Aug 18, 2022
View reviewed changes
packages/beacon-node/src/util/address.ts
@@ -1,3 +1,3 @@
export function isValidAddress(address: string): boolean {
return !!address && address.startsWith("0x") && address.length === 42;
return /^0x[0-9a-fA-F]{40}$/.test(address);
Copy link
Member

Choose a reason for hiding this comment

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

not sure how much this is used, I think it may be faster to create a singleton regex object, so it can be reused each time isValidAddress is called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NodeJS should cache those when interpreting the code (in theory)

@wemeetagain wemeetagain merged commit 0a1bccf into unstable Aug 19, 2022
@wemeetagain wemeetagain deleted the dapplion/isValidAddress branch August 19, 2022 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wemeetagain wemeetagain wemeetagain approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@dapplion @wemeetagain