Skip to content

Conversation

@Arjentix
Copy link
Contributor

Description

While original issues doesn't state it clearly I think that the problem was premature execution of time triggers.
So that trigger could be executed even before start. I fixed this.

  • Now it's forbidden to register schedule-based time-trigger with start point before latest_block_timestamp + consensus_estimation time, where consensus_estimation = 4 secs and is a hardcoded constant. Everithing before this timestamp is considered already analyzed by Iroha. In other words -- time-triggers in the past are forbidden.
  • As a consequence it's forbidden to register such trigger in genesis
  • Now premature time-trigger look up is forced to not to execute trigger start point is in future
  • This behavior is tested by a fixed change_asset_metadata_after_1_sec test

Forbidding time-triggers in the past was the simpliest solution and I actually think it was making sence to implement that in the initial solution.

Linked issue

Benefits

  • Less confusion about time-trigger execution time
  • No more painful triggers in the past

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@Arjentix Arjentix added Bug Something isn't working iroha2-dev The re-implementation of a BFT hyperledger in RUST labels Feb 27, 2024
@Arjentix Arjentix self-assigned this Feb 27, 2024
@github-actions github-actions bot added the api-changes Changes in the API for client libraries label Feb 27, 2024
@Arjentix Arjentix changed the title [fix] #3262: Don't prematurely execute time-triggers before their start timestamp` [fix] #3262: Don't prematurely execute time-triggers before their start timestamp Feb 27, 2024
@Arjentix Arjentix force-pushed the fix_time_trigger_first_execution_time branch from ff6b8ac to 160c838 Compare February 27, 2024 19:55
@coveralls
Copy link

Pull Request Test Coverage Report for Build 8070574387

Details

  • -16 of 20 (20.0%) changed or added relevant lines in 3 files are covered.
  • 3125 unchanged lines in 56 files lost coverage.
  • Overall coverage decreased (-0.3%) to 56.519%

Changes Missing Coverage Covered Lines Changed/Added Lines %
core/src/smartcontracts/isi/triggers/mod.rs 0 2 0.0%
data_model/src/isi.rs 0 2 0.0%
data_model/src/events/time.rs 4 16 25.0%
Files with Coverage Reduction New Missed Lines %
core/src/sumeragi/network_topology.rs 1 98.78%
data_model/src/events/time.rs 1 67.81%
tools/kagami/src/crypto.rs 1 12.12%
data_model/src/metadata.rs 2 78.87%
primitives/src/unique_vec.rs 2 91.24%
config/src/kura.rs 3 80.0%
config/src/logger.rs 5 72.73%
logger/src/lib.rs 5 94.12%
tools/kagami/src/main.rs 5 0.0%
crypto/src/signature/secp256k1.rs 7 96.09%
Totals Coverage Status
Change from base Build 7884695009: -0.3%
Covered Lines: 22455
Relevant Lines: 39730

💛 - Coveralls

@s8sato s8sato self-assigned this Feb 28, 2024
@VAmuzing VAmuzing self-assigned this Feb 29, 2024
@mversic mversic assigned mversic and unassigned Arjentix Mar 11, 2024
@nxsaken nxsaken changed the base branch from iroha2-dev to main April 16, 2024 08:31
@nxsaken nxsaken requested a review from dima74 as a code owner April 16, 2024 08:31
@Erigara Erigara self-assigned this May 7, 2024
@Erigara Erigara force-pushed the fix_time_trigger_first_execution_time branch from 160c838 to 5465b6a Compare May 13, 2024 12:32
@Erigara Erigara changed the title [fix] #3262: Don't prematurely execute time-triggers before their start timestamp fix: Don't prematurely execute time-triggers before their start timestamp May 13, 2024
@Erigara Erigara force-pushed the fix_time_trigger_first_execution_time branch from 5465b6a to e03ee2e Compare May 15, 2024 11:49
@mversic mversic requested a review from s8sato May 16, 2024 10:44
@Erigara Erigara force-pushed the fix_time_trigger_first_execution_time branch from e03ee2e to 68921fe Compare May 20, 2024 08:22
…estamp

Signed-off-by: Daniil Polyakov <[email protected]>
Signed-off-by: Shanin Roman <[email protected]>
@Erigara Erigara force-pushed the fix_time_trigger_first_execution_time branch from 68921fe to f032285 Compare May 22, 2024 06:32
@Erigara Erigara requested a review from VAmuzing May 22, 2024 07:25
@mversic mversic merged commit 7029eda into hyperledger-iroha:main May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-changes Changes in the API for client libraries Bug Something isn't working iroha2-dev The re-implementation of a BFT hyperledger in RUST

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[suggestion] Time trigger first execution time

6 participants