Bug/1702 adopt precompiled oracle #1713

olehnikolaiev · 2023-10-31T12:19:15Z

added two category of test in unit tests
the first one in ClientTest.cpp to test that precompiled contracts return different value depending on current block timestamp
the second one in PrecompiledTests.cpp to test that skaled properly parses historical data in config

tested manually on local machine as following:

deploy oracle contract
send oracle request and wait for response
hard-code response to the attached contract. it will submit the response to verifyOracleResponse function
send txn to the contract that do the checkOracleResponse call. see result
test.txt

to verify on qa network:

deploy oracle contract
stop one of the skaled containers
send oracle request and wait for response (you can use oracle-demo repo for it)
hard-code response to the attached contract. it will submit the response to verifyOracleResponse function
send txn to the contract that do the checkOracleResponse call. see result
do several node rotations on the chain
turn on the container that was stopped. wait till it catchup the txn with checkOracleResponse function and check its status.
wait for 2 snapshots interval to verify that stateRoot is the same on all nodes

github-actions · 2023-10-31T12:19:41Z

SUGGESTIONS BEFORE MERGE:

Build custom container here: https://github.com/skalenetwork/skaled/actions?query=workflow%3A%22Custom+Docker+and+Binary%22
Test it here: https://github.com/skalenetwork/skale-ci-integration_tests/actions?query=workflow%3A%22Skaled+Nightly+Tests%22
Edit VERSION file appropriately to your changes!
See https://skalelabs.atlassian.net/wiki/spaces/SKALE/pages/2444951553/Branching+instructions

codecov · 2023-11-01T12:23:54Z

Codecov Report

Merging #1713 (d301345) into v3.18.0 (9339f77) will increase coverage by 0.35%.
Report is 1 commits behind head on v3.18.0.
The diff coverage is 87.62%.

Additional details and impacted files

@@             Coverage Diff             @@
##           v3.18.0    #1713      +/-   ##
===========================================
+ Coverage    45.18%   45.53%   +0.35%     
===========================================
  Files          351      353       +2     
  Lines        51475    51633     +158     
===========================================
+ Hits         23258    23512     +254     
+ Misses       28217    28121      -96

kladkogex

Please use a patch mechanism to switch the code changes on at a particular block id

Also please provided a detailed explaination in pull request description how the pull request was tested

kladkogex

please either add detailed comments how the fix was made (preferred)
Or describe details of the fix in pull request

… reasons

kladkogex · 2023-11-07T12:59:12Z

libethereum/Precompiled.cpp

@@ -677,19 +679,21 @@ static const std::list< std::string > g_listReadableConfigParts{ "sealEngine",
    //"genesis.*"
    //"params.*",

-    "skaleConfig.nodeInfo.wallets.ima.commonBLSPublicKey*",
-    "skaleConfig.nodeInfo.wallets.ima.BLSPublicKey*",
+    // skaled-1702


Please remove the commented out section

According to the code guidelines we do not keep dead code

kladkogex · 2023-11-07T15:38:45Z

libethereum/Precompiled.cpp

@@ -677,19 +679,21 @@ static const std::list< std::string > g_listReadableConfigParts{ "sealEngine",
    //"genesis.*"


Please remove sealEngine parameter, since it is never used, and is always equal to ethHash

Please change stat_is_accessible_json_path fuction to use boost starts_with

kladkogex · 2023-11-07T15:42:03Z

libethereum/Precompiled.cpp

+    return boost::algorithm::starts_with( callData, "skaleConfig.sChain.nodes." );
+}
+
+static std::pair< std::string, unsigned > parseHistoricFieldReuqest( const std::string& callData ) {


Please improve so that exact matches are always used, so inccorect strings do not result in a match

kladkogex · 2023-11-07T15:44:19Z

libethereum/Precompiled.cpp

+        } else {
+            nlohmann::json joConfig = g_configAccesssor->getConfigJSON();
+            nlohmann::json joValue =
+                skutils::json_config_file_accessor::stat_extract_at_path( joConfig, rawName );
+            strValue = skutils::tools::trim_copy(
+                joValue.is_string() ? joValue.get< std::string >() : joValue.dump() );
+        }


Please change so that the else {} part is only used when the patch is enabled, since stat_extract_at_path is unsecure and complex. If it is needed, it needs to be rewritten in a clean and secure
way using standard C++ and boost libraries

kladkogex · 2023-11-07T15:46:22Z

libethereum/Precompiled.cpp

+            strValue = skutils::tools::trim_copy(
+                joValue.is_string() ? joValue.get< std::string >() : joValue.dump() );
+        }
+
        dev::u256 uValue( strValue.c_str() );


c_str() is not needed here since the constructor takes std::string

kladkogex · 2023-11-07T15:50:17Z

libethereum/Precompiled.cpp

@@ -677,19 +679,21 @@ static const std::list< std::string > g_listReadableConfigParts{ "sealEngine",
    //"genesis.*"
    //"params.*",


Please correct stat_parse_u256_hex_or_dec() function.

The constructor of u256 boost::multiprecision::number can take both decimal and hex string and autodetect.

Therefore, this function can be simplified to just calling constructor of u256

libethereum/Precompiled.cpp

…etwork/skaled into bug/1702-adopt-precompiled-oracle

olehnikolaiev added 5 commits October 20, 2023 17:02

1702 add owner field to nodes in chain params

685fd91

1702 use historicGroupIndex

a733b76

1702 update precompiled to work with oracle

b7cd5a5

1702 improve tests

295b850

1702 improve tests

786882f

olehnikolaiev requested review from DmytroNazarenko and kladkogex as code owners October 31, 2023 12:19

olehnikolaiev and others added 3 commits October 31, 2023 15:18

1702 remove unnecessary changes

e9b63e9

Merge branch 'v3.18.0' into bug/1702-adopt-precompiled-oracle

4f22ffb

1702 fix tests

a348f52

olehnikolaiev linked an issue Nov 1, 2023 that may be closed by this pull request

adopt some of the precompiled contracts to process schain groups logic #1702

Closed

kladkogex requested changes Nov 3, 2023

View reviewed changes

1702 introduce PrecompiledConfigPatch

3815c22

kladkogex requested changes Nov 3, 2023

View reviewed changes

kladkogex and others added 10 commits November 6, 2023 13:04

#1702 renamed owner to nodeAddress

ee9c48a

#1702 changed [] to at() for safe vector access

aa4c0d8

#1702 added param validation

95215c7

#1702 added a log record if node info is empty in config

4d55443

#1702 changed assertion to runtime error

7f136c5

#1702 changed assertion to runtime error

ea28913

#1702 changed to standard boost library

3652a82

1702 fix build

41ddce9

1702 deny access to some config fields from precompileds for security…

399e9b1

… reasons

1702 fix tests

cfbd435

kladkogex force-pushed the bug/1702-adopt-precompiled-oracle branch from df1aad9 to cfbd435 Compare November 7, 2023 15:31

olehnikolaiev added 3 commits November 7, 2023 18:53

#1702 improve code quality

80618dd

#1702 use common approach to access config variables

582b526

#1702 improve code quality

ea694fc

kladkogex previously approved these changes Nov 8, 2023

View reviewed changes

#1702 improve input proccessing for precompileds

ca16ebf

olehnikolaiev dismissed kladkogex’s stale review via ca16ebf November 13, 2023 13:35

#1702 add more checks

4995613

dimalit previously approved these changes Nov 13, 2023

View reviewed changes

#1702 improve tests

edd62d4

olehnikolaiev dismissed dimalit’s stale review via edd62d4 November 13, 2023 17:44

olehnikolaiev requested review from dimalit and kladkogex November 13, 2023 17:47

Merge branch 'v3.18.0' into bug/1702-adopt-precompiled-oracle

aaab497

kladkogex previously approved these changes Nov 21, 2023

View reviewed changes

olehnikolaiev and others added 3 commits November 21, 2023 13:30

Merge branch 'v3.18.0' into bug/1702-adopt-precompiled-oracle

3b122e5

#1702 pass historic publicKey instead of address

4b51081

Merge branch 'bug/1702-adopt-precompiled-oracle' of github.com:skalen…

e411718

…etwork/skaled into bug/1702-adopt-precompiled-oracle

olehnikolaiev dismissed kladkogex’s stale review via e411718 November 22, 2023 11:47

olehnikolaiev added 3 commits November 22, 2023 13:58

#1702 fix tests

a504090

#1702 add more tests

1a67380

#1702 change [] to at()

7700738

dimalit previously approved these changes Nov 22, 2023

View reviewed changes

#1702 change [] to at()

c8921f6

olehnikolaiev dismissed dimalit’s stale review via c8921f6 November 22, 2023 17:56

#1702 change [] to at()

23a5437

DmytroNazarenko approved these changes Nov 22, 2023

View reviewed changes

dimalit approved these changes Nov 22, 2023

View reviewed changes

Merge branch 'v3.18.0' into bug/1702-adopt-precompiled-oracle

d301345

kladkogex self-requested a review November 23, 2023 16:29

kladkogex approved these changes Nov 23, 2023

View reviewed changes

olehnikolaiev merged commit 1de9f45 into v3.18.0 Nov 23, 2023
8 checks passed

olehnikolaiev deleted the bug/1702-adopt-precompiled-oracle branch November 23, 2023 16:37

github-actions bot locked and limited conversation to collaborators Nov 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug/1702 adopt precompiled oracle #1713

Bug/1702 adopt precompiled oracle #1713

olehnikolaiev commented Oct 31, 2023 •

edited

Loading

github-actions bot commented Oct 31, 2023

codecov bot commented Nov 1, 2023 •

edited

Loading

kladkogex left a comment

kladkogex left a comment

kladkogex Nov 7, 2023

kladkogex Nov 7, 2023

kladkogex Nov 7, 2023

kladkogex Nov 7, 2023

kladkogex Nov 7, 2023

kladkogex Nov 7, 2023

kladkogex Nov 7, 2023

		@@ -677,19 +679,21 @@ static const std::list< std::string > g_listReadableConfigParts{ "sealEngine",
		//"genesis.*"

		@@ -677,19 +679,21 @@ static const std::list< std::string > g_listReadableConfigParts{ "sealEngine",
		//"genesis.*"
		//"params.*",

Bug/1702 adopt precompiled oracle #1713

Bug/1702 adopt precompiled oracle #1713

Conversation

olehnikolaiev commented Oct 31, 2023 • edited Loading

github-actions bot commented Oct 31, 2023

codecov bot commented Nov 1, 2023 • edited Loading

Codecov Report

kladkogex left a comment

Choose a reason for hiding this comment

kladkogex left a comment

Choose a reason for hiding this comment

kladkogex Nov 7, 2023

Choose a reason for hiding this comment

kladkogex Nov 7, 2023

Choose a reason for hiding this comment

kladkogex Nov 7, 2023

Choose a reason for hiding this comment

kladkogex Nov 7, 2023

Choose a reason for hiding this comment

kladkogex Nov 7, 2023

Choose a reason for hiding this comment

kladkogex Nov 7, 2023

Choose a reason for hiding this comment

kladkogex Nov 7, 2023

Choose a reason for hiding this comment

olehnikolaiev commented Oct 31, 2023 •

edited

Loading

codecov bot commented Nov 1, 2023 •

edited

Loading