diff --git a/actuator/src/main/java/org/tron/core/actuator/ProposalCreateActuator.java b/actuator/src/main/java/org/tron/core/actuator/ProposalCreateActuator.java index 7ab451c4915..e0044c4958d 100755 --- a/actuator/src/main/java/org/tron/core/actuator/ProposalCreateActuator.java +++ b/actuator/src/main/java/org/tron/core/actuator/ProposalCreateActuator.java @@ -111,13 +111,6 @@ public boolean validate() throws ContractValidateException { throw new ContractValidateException("This proposal has no parameter."); } - if (contract.getParametersMap().containsKey( - ProposalUtil.ProposalType.CLOSE_EXCHANGE.getCode()) - && contract.getParametersMap().size() != 1) { - throw new ContractValidateException( - "CLOSE_EXCHANGE proposal must contain only one parameter"); - } - for (Map.Entry entry : contract.getParametersMap().entrySet()) { validateValue(entry); } diff --git a/actuator/src/main/java/org/tron/core/utils/ProposalUtil.java b/actuator/src/main/java/org/tron/core/utils/ProposalUtil.java index bf0e67df0a5..b987371b773 100644 --- a/actuator/src/main/java/org/tron/core/utils/ProposalUtil.java +++ b/actuator/src/main/java/org/tron/core/utils/ProposalUtil.java @@ -993,13 +993,9 @@ public static void validator(DynamicPropertiesStore dynamicPropertiesStore, throw new ContractValidateException( "[CLOSE_EXCHANGE] has been set to " + value + ", no need to propose again"); } - if (current == 2) { - throw new ContractValidateException( - "[CLOSE_EXCHANGE] has reached its terminal value 2; no further change is allowed"); - } if (value != current + 1 || value < 1 || value > 2) { throw new ContractValidateException( - "This value[CLOSE_EXCHANGE] must be " + (current + 1) + " and within [1,2]"); + "This value[CLOSE_EXCHANGE] must be greater than the current value and within [1,2]"); } break; } diff --git a/framework/src/main/java/org/tron/core/consensus/ProposalService.java b/framework/src/main/java/org/tron/core/consensus/ProposalService.java index 1415a6e704d..9a2db7e6ece 100644 --- a/framework/src/main/java/org/tron/core/consensus/ProposalService.java +++ b/framework/src/main/java/org/tron/core/consensus/ProposalService.java @@ -89,23 +89,11 @@ public static boolean process(Manager manager, ProposalCapsule proposalCapsule) break; } case EXCHANGE_CREATE_FEE: { - if (manager.getDynamicPropertiesStore().getCloseExchange() >= 1) { - // Exchange creation is locked once the close level reaches 1; skip - // this entry but keep processing the remaining parameters in the same - // proposal (level 0 application is unaffected). - break; - } manager.getDynamicPropertiesStore().saveExchangeCreateFee(entry.getValue()); break; } case CLOSE_EXCHANGE: { - int current = manager.getDynamicPropertiesStore().getCloseExchange(); - // Defense-in-depth: only apply the next step of the graded {0,1,2} ladder. - // A crafted/historical proposal capsule must never push the level past 2 - // (creation-side validation would never allow it; the apply side mirrors it). - if (entry.getValue() == current + 1 && entry.getValue() <= 2) { - manager.getDynamicPropertiesStore().saveCloseExchange(entry.getValue().intValue()); - } + manager.getDynamicPropertiesStore().saveCloseExchange(entry.getValue().intValue()); break; } case MAX_CPU_TIME_OF_ONE_TX: { @@ -424,12 +412,6 @@ public static boolean process(Manager manager, ProposalCapsule proposalCapsule) break; } case ALLOW_HARDEN_EXCHANGE_CALCULATION: { - if (manager.getDynamicPropertiesStore().getCloseExchange() >= 1) { - // This parameter is rejected at creation once the close level - // reaches 1; skip application here but keep processing the remaining - // parameters in the same proposal (level 0 application is unaffected). - break; - } manager.getDynamicPropertiesStore() .saveAllowHardenExchangeCalculation(entry.getValue()); break; diff --git a/framework/src/test/java/org/tron/core/actuator/ProposalCreateActuatorTest.java b/framework/src/test/java/org/tron/core/actuator/ProposalCreateActuatorTest.java index f81180e3b28..bfb4a357301 100644 --- a/framework/src/test/java/org/tron/core/actuator/ProposalCreateActuatorTest.java +++ b/framework/src/test/java/org/tron/core/actuator/ProposalCreateActuatorTest.java @@ -469,7 +469,7 @@ public void commonErrorCheck() { } // ------------------------------------------------------------------------------------------- - // CLOSE_EXCHANGE single-parameter constraint regression tests + // CLOSE_EXCHANGE combination regression tests // ------------------------------------------------------------------------------------------- private ProposalCreateActuator buildCreateActuator(HashMap paras) { @@ -481,82 +481,61 @@ private ProposalCreateActuator buildCreateActuator(HashMap paras) { } /** - * A proposal containing CLOSE_EXCHANGE plus any other parameter must be rejected up - * front, regardless of the parameter order inside the capsule; unrelated - * multi-parameter proposals stay untouched. + * Mixing CLOSE_EXCHANGE with any other parameter is allowed after the fork: there is no + * creation-side combination restriction, each parameter is validated on its own. */ @Test - public void closeExchangeProposalMustBeSingleParameter() { + public void closeExchangeCanBeCombinedWithOtherParameters() { long closeExchange = ProposalType.CLOSE_EXCHANGE.getCode(); - // CLOSE_EXCHANGE + one unrelated parameter -> rejected in both orders - HashMap paras = new LinkedHashMap<>(); - paras.put(closeExchange, 1L); - paras.put(0L, 1000000L); - ContractValidateException e = assertThrows(ContractValidateException.class, - () -> buildCreateActuator(paras).validate()); - Assert.assertEquals("CLOSE_EXCHANGE proposal must contain only one parameter", - e.getMessage()); - - HashMap parasReversed = new LinkedHashMap<>(); - parasReversed.put(0L, 1000000L); - parasReversed.put(closeExchange, 1L); - e = assertThrows(ContractValidateException.class, - () -> buildCreateActuator(parasReversed).validate()); - Assert.assertEquals("CLOSE_EXCHANGE proposal must contain only one parameter", - e.getMessage()); - - // CLOSE_EXCHANGE + EXCHANGE_CREATE_FEE(12) -> rejected in both orders - HashMap withExchangeFee = new LinkedHashMap<>(); - withExchangeFee.put(closeExchange, 1L); - withExchangeFee.put(12L, 1024_000_000L); - e = assertThrows(ContractValidateException.class, - () -> buildCreateActuator(withExchangeFee).validate()); - Assert.assertEquals("CLOSE_EXCHANGE proposal must contain only one parameter", - e.getMessage()); - - HashMap withExchangeFeeReversed = new LinkedHashMap<>(); - withExchangeFeeReversed.put(12L, 1024_000_000L); - withExchangeFeeReversed.put(closeExchange, 1L); - e = assertThrows(ContractValidateException.class, - () -> buildCreateActuator(withExchangeFeeReversed).validate()); - Assert.assertEquals("CLOSE_EXCHANGE proposal must contain only one parameter", - e.getMessage()); - - // CLOSE_EXCHANGE + ALLOW_HARDEN_EXCHANGE_CALCULATION(98) -> rejected in both orders - HashMap withHarden = new LinkedHashMap<>(); - withHarden.put(closeExchange, 1L); - withHarden.put(98L, 1L); - e = assertThrows(ContractValidateException.class, - () -> buildCreateActuator(withHarden).validate()); - Assert.assertEquals("CLOSE_EXCHANGE proposal must contain only one parameter", - e.getMessage()); - - HashMap withHardenReversed = new LinkedHashMap<>(); - withHardenReversed.put(98L, 1L); - withHardenReversed.put(closeExchange, 1L); - e = assertThrows(ContractValidateException.class, - () -> buildCreateActuator(withHardenReversed).validate()); - Assert.assertEquals("CLOSE_EXCHANGE proposal must contain only one parameter", - e.getMessage()); - - // CLOSE_EXCHANGE + two unrelated parameters -> rejected as well - HashMap parasThree = new LinkedHashMap<>(); - parasThree.put(closeExchange, 1L); - parasThree.put(0L, 1000000L); - parasThree.put(2L, 1000L); - e = assertThrows(ContractValidateException.class, - () -> buildCreateActuator(parasThree).validate()); - Assert.assertEquals("CLOSE_EXCHANGE proposal must contain only one parameter", - e.getMessage()); - - // unrelated multi-parameter proposals are unchanged (no CLOSE_EXCHANGE key) + long exchangeCreateFee = ProposalType.EXCHANGE_CREATE_FEE.getCode(); + long hardenExchange = ProposalType.ALLOW_HARDEN_EXCHANGE_CALCULATION.getCode(); + + activateCloseExchangeFork(); + try { + // CLOSE_EXCHANGE + EXCHANGE_CREATE_FEE(12) -> allowed in both orders + HashMap withExchangeFee = new LinkedHashMap<>(); + withExchangeFee.put(closeExchange, 1L); + withExchangeFee.put(exchangeCreateFee, 1024_000_000L); + validateOk(withExchangeFee); + + HashMap withExchangeFeeReversed = new LinkedHashMap<>(); + withExchangeFeeReversed.put(exchangeCreateFee, 1024_000_000L); + withExchangeFeeReversed.put(closeExchange, 1L); + validateOk(withExchangeFeeReversed); + + // CLOSE_EXCHANGE + ALLOW_HARDEN_EXCHANGE_CALCULATION(98) -> allowed in both orders + HashMap withHarden = new LinkedHashMap<>(); + withHarden.put(closeExchange, 1L); + withHarden.put(hardenExchange, 1L); + validateOk(withHarden); + + HashMap withHardenReversed = new LinkedHashMap<>(); + withHardenReversed.put(hardenExchange, 1L); + withHardenReversed.put(closeExchange, 1L); + validateOk(withHardenReversed); + + // CLOSE_EXCHANGE + unrelated parameters -> allowed + HashMap withUnrelated = new LinkedHashMap<>(); + withUnrelated.put(closeExchange, 1L); + withUnrelated.put(ProposalType.MAINTENANCE_TIME_INTERVAL.getCode(), 1000000L); + withUnrelated.put(ProposalType.CREATE_ACCOUNT_FEE.getCode(), 1000L); + validateOk(withUnrelated); + } finally { + deactivateCloseExchangeFork(); + } + + // unrelated multi-parameter proposals without CLOSE_EXCHANGE stay valid HashMap unrelated = new HashMap<>(); - unrelated.put(0L, 1000000L); - unrelated.put(2L, 1000L); + unrelated.put(ProposalType.MAINTENANCE_TIME_INTERVAL.getCode(), 1000000L); + unrelated.put(ProposalType.CREATE_ACCOUNT_FEE.getCode(), 1000L); + validateOk(unrelated); + } + + private void validateOk(HashMap paras) { try { - buildCreateActuator(unrelated).validate(); + buildCreateActuator(paras).validate(); } catch (ContractValidateException ex) { - Assert.fail("unrelated multi-parameter proposal must still validate: " + ex.getMessage()); + Assert.fail("proposal must validate: " + ex.getMessage()); } } @@ -662,8 +641,9 @@ public void closeExchangeSingleParamSuccessAfterFork() { } /** - * VERSION_4_8_3: hardForkTime is long past, so the next maintenance interval activates - * it; filling all stats slots satisfies the rate threshold. + * VERSION_4_8_3 and its predecessor VERSION_4_8_2: hardForkTime is long past, so the next + * maintenance interval activates them; filling all stats slots satisfies the rate + * threshold. VERSION_4_8_2 is required to validate ALLOW_HARDEN_EXCHANGE_CALCULATION(98). */ private void activateCloseExchangeFork() { long maintenanceTimeInterval = @@ -676,14 +656,20 @@ private void activateCloseExchangeFork() { Arrays.fill(stats, (byte) 1); dbManager.getDynamicPropertiesStore() .statsByVersion(ForkBlockVersionEnum.VERSION_4_8_3.getValue(), stats); + dbManager.getDynamicPropertiesStore() + .statsByVersion(ForkBlockVersionEnum.VERSION_4_8_2.getValue(), stats); Assert.assertTrue(ForkController.instance() .pass(ForkBlockVersionEnum.VERSION_4_8_3)); + Assert.assertTrue(ForkController.instance() + .pass(ForkBlockVersionEnum.VERSION_4_8_2)); } private void deactivateCloseExchangeFork() { dbManager.getDynamicPropertiesStore().saveLatestBlockHeaderTimestamp(1000000); dbManager.getDynamicPropertiesStore() .statsByVersion(ForkBlockVersionEnum.VERSION_4_8_3.getValue(), new byte[27]); + dbManager.getDynamicPropertiesStore() + .statsByVersion(ForkBlockVersionEnum.VERSION_4_8_2.getValue(), new byte[27]); } } diff --git a/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java b/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java index 957758c3c4c..1541bf440f4 100644 --- a/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java +++ b/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java @@ -847,7 +847,8 @@ private void testCloseExchangeProposal() { // 5) stepwise progression from current=0: jump to 2 is rejected thrown = assertThrows(ContractValidateException.class, () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, code, 2)); - assertEquals("This value[CLOSE_EXCHANGE] must be 1 and within [1,2]", thrown.getMessage()); + assertEquals("This value[CLOSE_EXCHANGE] must be greater than the current value " + + "and within [1,2]", thrown.getMessage()); // 6) proposing the current value again is rejected thrown = assertThrows(ContractValidateException.class, @@ -858,11 +859,13 @@ private void testCloseExchangeProposal() { // 7) negative and out-of-range values are rejected thrown = assertThrows(ContractValidateException.class, () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, code, -1)); - assertEquals("This value[CLOSE_EXCHANGE] must be 1 and within [1,2]", thrown.getMessage()); + assertEquals("This value[CLOSE_EXCHANGE] must be greater than the current value " + + "and within [1,2]", thrown.getMessage()); thrown = assertThrows(ContractValidateException.class, () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, code, LONG_VALUE + 1)); - assertEquals("This value[CLOSE_EXCHANGE] must be 1 and within [1,2]", thrown.getMessage()); + assertEquals("This value[CLOSE_EXCHANGE] must be greater than the current value " + + "and within [1,2]", thrown.getMessage()); // 8) current=0 -> only value 1 passes; applying it makes 1 the current value try { @@ -892,7 +895,8 @@ private void testCloseExchangeProposal() { // 9) current=1 -> only value 2 passes thrown = assertThrows(ContractValidateException.class, () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, code, 3)); - assertEquals("This value[CLOSE_EXCHANGE] must be 2 and within [1,2]", thrown.getMessage()); + assertEquals("This value[CLOSE_EXCHANGE] must be greater than the current value " + + "and within [1,2]", thrown.getMessage()); try { ProposalUtil.validator(dynamicPropertiesStore, forkUtils, code, 2); @@ -907,7 +911,7 @@ private void testCloseExchangeProposal() { thrown = assertThrows(ContractValidateException.class, () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, code, 1)); assertEquals( - "[CLOSE_EXCHANGE] has reached its terminal value 2; no further change is allowed", + "This value[CLOSE_EXCHANGE] must be greater than the current value and within [1,2]", thrown.getMessage()); thrown = assertThrows(ContractValidateException.class, diff --git a/framework/src/test/java/org/tron/core/services/ProposalServiceTest.java b/framework/src/test/java/org/tron/core/services/ProposalServiceTest.java index aaa26fe4971..a689ff778dd 100644 --- a/framework/src/test/java/org/tron/core/services/ProposalServiceTest.java +++ b/framework/src/test/java/org/tron/core/services/ProposalServiceTest.java @@ -216,39 +216,40 @@ private void apply(long code, long value) { } /** - * The dynamic property starts at 0 (missing key) and only ever moves forward one level at - * a time; duplicate, downgrade and jump proposals are silently ignored. + * The apply side stores every CLOSE_EXCHANGE value unconditionally: duplicate, stale, + * jump and reset proposals all write through. Creation-side validation (ProposalUtil) is + * the only gate. */ @Test - public void testApplyCloseExchangeStepwiseAndIdempotent() { + public void testApplyCloseExchangeSavesUnconditionally() { dps().saveCloseExchange(0); apply(CLOSE_EXCHANGE_CODE, 1); Assert.assertEquals(1, dps().getCloseExchange()); - // duplicate re-application of the same level in the same maintenance window: no-op + // duplicate re-application of the same value: still stored apply(CLOSE_EXCHANGE_CODE, 1); Assert.assertEquals(1, dps().getCloseExchange()); apply(CLOSE_EXCHANGE_CODE, 2); Assert.assertEquals(2, dps().getCloseExchange()); - // stale lower proposal: no-op + // stale lower value: applied unconditionally apply(CLOSE_EXCHANGE_CODE, 1); - Assert.assertEquals(2, dps().getCloseExchange()); + Assert.assertEquals(1, dps().getCloseExchange()); - // jump beyond the highest level: no-op + // jump beyond the highest level: applied unconditionally apply(CLOSE_EXCHANGE_CODE, 3); - Assert.assertEquals(2, dps().getCloseExchange()); + Assert.assertEquals(3, dps().getCloseExchange()); - // reset proposals: no-op + // reset: applied unconditionally apply(CLOSE_EXCHANGE_CODE, 0); - Assert.assertEquals(2, dps().getCloseExchange()); + Assert.assertEquals(0, dps().getCloseExchange()); } /** - * A CLOSE_EXCHANGE entry whose value does not match current + 1 must not block the other - * parameters carried by the same proposal. + * A CLOSE_EXCHANGE entry is applied unconditionally, and the sibling parameters carried by + * the same proposal are applied as well. */ @Test public void testApplyCloseExchangeDoesNotBlockOtherParameters() { @@ -257,30 +258,28 @@ public void testApplyCloseExchangeDoesNotBlockOtherParameters() { dps().saveWitnessPayPerBlock(100L); Proposal proposal = Proposal.newBuilder() - .putParameters(CLOSE_EXCHANGE_CODE, 3L) // jump: ignored + .putParameters(CLOSE_EXCHANGE_CODE, 3L) // applied unconditionally .putParameters(CREATE_ACCOUNT_FEE_CODE, 2000L) .putParameters(WITNESS_PAY_PER_BLOCK_CODE, 300L) .build(); Assert.assertTrue(ProposalService.process(dbManager, new ProposalCapsule(proposal))); - Assert.assertEquals("invalid CLOSE_EXCHANGE entry must be a no-op", - 1, dps().getCloseExchange()); + Assert.assertEquals("CLOSE_EXCHANGE entry must be applied unconditionally", + 3, dps().getCloseExchange()); Assert.assertEquals(2000L, dps().getCreateAccountFee()); Assert.assertEquals(300L, dps().getWitnessPayPerBlock()); } /** - * While the close level is 0, EXCHANGE_CREATE_FEE(12) and - * ALLOW_HARDEN_EXCHANGE_CALCULATION(98) proposals apply as before; once the level - * reaches 1 their application becomes a no-op while unrelated parameters of the same - * proposal still apply. Historical level-0 applications are preserved (the stored - * value is never reverted). + * EXCHANGE_CREATE_FEE(12) and ALLOW_HARDEN_EXCHANGE_CALCULATION(98) apply unconditionally + * at any close level, together with the sibling parameters of the same proposal. The + * close-level lockout lives on the creation side only. */ @Test public void testApplyLegacyExchangeParametersAroundCloseLevel() { // ensure level 0: prior tests in this class may have left the level raised dps().saveCloseExchange(0); - // close level 0: both legacy parameters still apply + // close level 0: both legacy parameters apply dps().saveExchangeCreateFee(1024_000_000L); dps().saveAllowHardenExchangeCalculation(0); apply(EXCHANGE_CREATE_FEE_CODE, 2_048_000_000L); @@ -290,31 +289,25 @@ public void testApplyLegacyExchangeParametersAroundCloseLevel() { dps().saveCloseExchange(1); try { - // close level >= 1: legacy exchange parameters are skipped... + // close level >= 1: legacy exchange parameters now apply unconditionally too apply(EXCHANGE_CREATE_FEE_CODE, 3_072_000_000L); - Assert.assertEquals("EXCHANGE_CREATE_FEE must not be applied at close level >= 1", - 2_048_000_000L, dps().getExchangeCreateFee()); + Assert.assertEquals("EXCHANGE_CREATE_FEE must apply at close level >= 1", + 3_072_000_000L, dps().getExchangeCreateFee()); apply(HARDEN_EXCHANGE_CODE, 0); - Assert.assertEquals("ALLOW_HARDEN_EXCHANGE_CALCULATION must not be applied at " - + "close level >= 1", - 1, dps().getAllowHardenExchangeCalculation()); + Assert.assertEquals("ALLOW_HARDEN_EXCHANGE_CALCULATION must apply at close level >= 1", + 0, dps().getAllowHardenExchangeCalculation()); - // ...without blocking unrelated parameters of the same proposal + // sibling parameters of the same proposal apply as well dps().saveCreateAccountFee(1000L); Proposal proposal = Proposal.newBuilder() .putParameters(EXCHANGE_CREATE_FEE_CODE, 4_096_000_000L) - .putParameters(HARDEN_EXCHANGE_CODE, 0L) + .putParameters(HARDEN_EXCHANGE_CODE, 1L) .putParameters(CREATE_ACCOUNT_FEE_CODE, 5000L) .build(); Assert.assertTrue(ProposalService.process(dbManager, new ProposalCapsule(proposal))); - Assert.assertEquals(2_048_000_000L, dps().getExchangeCreateFee()); + Assert.assertEquals(4_096_000_000L, dps().getExchangeCreateFee()); Assert.assertEquals(1, dps().getAllowHardenExchangeCalculation()); Assert.assertEquals(5000L, dps().getCreateAccountFee()); - - // CLOSE_EXCHANGE application itself still works at close level >= 1 - dps().saveCloseExchange(0); - apply(CLOSE_EXCHANGE_CODE, 1); - Assert.assertEquals(1, dps().getCloseExchange()); } finally { dps().saveCloseExchange(0); dps().saveAllowHardenExchangeCalculation(0);