This repository has been archived by the owner on Sep 26, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 130
[PAN-3126] add an override facility for genesis configs #1915
Merged
+229
−35
Merged
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
679e70e
add an override facility for genesis configs
shemnon 741e7dd
error prone
shemnon d2ec9f0
refactor missed some mocks
shemnon 684216a
spotless
shemnon 88153cb
one more mockito change
shemnon c4f98db
one more any()
shemnon c23bd25
review comments
shemnon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,10 +15,12 @@ | |
import static java.util.Objects.isNull; | ||
|
||
import java.math.BigInteger; | ||
import java.util.Collections; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.OptionalInt; | ||
import java.util.OptionalLong; | ||
import java.util.TreeMap; | ||
|
||
import com.fasterxml.jackson.databind.node.ObjectNode; | ||
import com.google.common.collect.ImmutableMap; | ||
|
@@ -30,13 +32,22 @@ public class JsonGenesisConfigOptions implements GenesisConfigOptions { | |
private static final String IBFT2_CONFIG_KEY = "ibft2"; | ||
private static final String CLIQUE_CONFIG_KEY = "clique"; | ||
private final ObjectNode configRoot; | ||
private final Map<String, String> configOverrides = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); | ||
|
||
public static JsonGenesisConfigOptions fromJsonObject(final ObjectNode configRoot) { | ||
return new JsonGenesisConfigOptions(configRoot); | ||
} | ||
|
||
JsonGenesisConfigOptions(final ObjectNode maybeConfig) { | ||
private JsonGenesisConfigOptions(final ObjectNode maybeConfig) { | ||
this(maybeConfig, Collections.emptyMap()); | ||
} | ||
|
||
JsonGenesisConfigOptions( | ||
final ObjectNode maybeConfig, final Map<String, String> configOverrides) { | ||
this.configRoot = isNull(maybeConfig) ? JsonUtil.createEmptyObjectNode() : maybeConfig; | ||
if (configOverrides != null) { | ||
this.configOverrides.putAll(configOverrides); | ||
} | ||
} | ||
|
||
@Override | ||
|
@@ -148,17 +159,17 @@ public OptionalLong getIstanbulBlockNumber() { | |
|
||
@Override | ||
public Optional<BigInteger> getChainId() { | ||
return JsonUtil.getValueAsString(configRoot, "chainid").map(BigInteger::new); | ||
return getOptionalBigInteger("chainid"); | ||
} | ||
|
||
@Override | ||
public OptionalInt getContractSizeLimit() { | ||
return JsonUtil.getInt(configRoot, "contractsizelimit"); | ||
return getOptionalInt("contractsizelimit"); | ||
} | ||
|
||
@Override | ||
public OptionalInt getEvmStackSize() { | ||
return JsonUtil.getInt(configRoot, "evmstacksize"); | ||
return getOptionalInt("evmstacksize"); | ||
} | ||
|
||
@Override | ||
|
@@ -176,9 +187,8 @@ public Map<String, Object> asMap() { | |
.ifPresent( | ||
l -> { | ||
builder.put("eip150Block", l); | ||
if (configRoot.has("eip150hash")) { | ||
builder.put("eip150Hash", configRoot.get("eip150hash").asText()); | ||
} | ||
getOptionalString("eip150hash") | ||
.ifPresent(eip150hash -> builder.put("eip150Hash", eip150hash)); | ||
}); | ||
getSpuriousDragonBlockNumber() | ||
.ifPresent( | ||
|
@@ -207,7 +217,45 @@ public Map<String, Object> asMap() { | |
return builder.build(); | ||
} | ||
|
||
private Optional<String> getOptionalString(final String key) { | ||
if (configOverrides.containsKey(key)) { | ||
final String value = configOverrides.get(key); | ||
return value == null || value.isEmpty() ? Optional.empty() : Optional.of(value); | ||
} else { | ||
return JsonUtil.getString(configRoot, key); | ||
} | ||
} | ||
|
||
private OptionalLong getOptionalLong(final String key) { | ||
return JsonUtil.getLong(configRoot, key); | ||
if (configOverrides.containsKey(key)) { | ||
final String value = configOverrides.get(key); | ||
return value == null || value.isEmpty() | ||
? OptionalLong.empty() | ||
: OptionalLong.of(Long.valueOf(configOverrides.get(key))); | ||
} else { | ||
return JsonUtil.getLong(configRoot, key); | ||
} | ||
} | ||
|
||
private OptionalInt getOptionalInt(final String key) { | ||
if (configOverrides.containsKey(key)) { | ||
final String value = configOverrides.get(key); | ||
return value == null || value.isEmpty() | ||
? OptionalInt.empty() | ||
: OptionalInt.of(Integer.valueOf(configOverrides.get(key))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we explicitly specify the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
} else { | ||
return JsonUtil.getInt(configRoot, key); | ||
} | ||
} | ||
|
||
private Optional<BigInteger> getOptionalBigInteger(final String key) { | ||
if (configOverrides.containsKey(key)) { | ||
final String value = configOverrides.get(key); | ||
return value == null || value.isEmpty() | ||
? Optional.empty() | ||
: Optional.of(new BigInteger(value)); | ||
} else { | ||
return JsonUtil.getValueAsString(configRoot, key).map(BigInteger::new); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
|
||
import java.math.BigInteger; | ||
import java.util.Map; | ||
import java.util.TreeMap; | ||
import java.util.function.Function; | ||
import java.util.stream.Collectors; | ||
|
||
|
@@ -214,6 +215,59 @@ public void acceptComments() { | |
// Unfortunately there is no good (non-flakey) way to assert logs. | ||
} | ||
|
||
@Test | ||
public void testOverridePresent() { | ||
final GenesisConfigFile config = GenesisConfigFile.development(); | ||
final int bigBlock = 999_999_999; | ||
final String bigBlockString = Integer.toString(bigBlock); | ||
final Map<String, String> override = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add some tests that use a case-sensitive There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
override.put("istanbulBlock", bigBlockString); | ||
override.put("chainId", bigBlockString); | ||
override.put("contractSizeLimit", bigBlockString); | ||
|
||
assertThat(config.getConfigOptions(override).getIstanbulBlockNumber()).hasValue(bigBlock); | ||
assertThat(config.getConfigOptions(override).getChainId()) | ||
.hasValue(BigInteger.valueOf(bigBlock)); | ||
assertThat(config.getConfigOptions(override).getContractSizeLimit()).hasValue(bigBlock); | ||
} | ||
|
||
@Test | ||
public void testOverrideNull() { | ||
final GenesisConfigFile config = GenesisConfigFile.development(); | ||
final Map<String, String> override = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); | ||
override.put("istanbulBlock", null); | ||
override.put("chainId", null); | ||
override.put("contractSizeLimit", null); | ||
|
||
assertThat(config.getConfigOptions(override).getIstanbulBlockNumber()).isNotPresent(); | ||
assertThat(config.getConfigOptions(override).getChainId()).isNotPresent(); | ||
assertThat(config.getConfigOptions(override).getContractSizeLimit()).isNotPresent(); | ||
} | ||
|
||
@Test | ||
public void testOverrideEmptyString() { | ||
final GenesisConfigFile config = GenesisConfigFile.development(); | ||
final Map<String, String> override = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); | ||
override.put("istanbulBlock", ""); | ||
override.put("chainId", ""); | ||
override.put("contractSizeLimit", ""); | ||
|
||
assertThat(config.getConfigOptions(override).getIstanbulBlockNumber()).isNotPresent(); | ||
assertThat(config.getConfigOptions(override).getChainId()).isNotPresent(); | ||
assertThat(config.getConfigOptions(override).getContractSizeLimit()).isNotPresent(); | ||
} | ||
|
||
@Test | ||
public void testNoOverride() { | ||
final GenesisConfigFile config = GenesisConfigFile.development(); | ||
|
||
assertThat(config.getConfigOptions().getConstantinopleFixBlockNumber()).hasValue(0); | ||
assertThat(config.getConfigOptions().getIstanbulBlockNumber()).isNotPresent(); | ||
assertThat(config.getConfigOptions().getChainId()).hasValue(BigInteger.valueOf(2018)); | ||
assertThat(config.getConfigOptions().getContractSizeLimit()).hasValue(2147483647); | ||
assertThat(config.getConfigOptions().getEvmStackSize()).isNotPresent(); | ||
} | ||
|
||
private GenesisConfigFile configWithProperty(final String key, final String value) { | ||
return GenesisConfigFile.fromConfig("{\"" + key + "\":\"" + value + "\"}"); | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we explicitly specify the
radix
here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done