S8 NN Update preparations #47

rumeysayilmaz · 2024-09-18T08:01:37Z

This PR includes the changes for s8 NN update. We kindly request @DeckerSU and @dimxy to review the changes. Thank you in advance.

Regarding the changes

Should nS8HardforkHeight be calculated according to marmarachain blocks?
Should the NotaryChecker Class added in here?

modified: src/clientversion.h -> updated version to 1.2.1 modified: src/komodo_defs.h -> dpow s8 nn pubkeys and season definiitons and variables added modified: src/komodo_globals.h -> s8 hf height added modified: src/version.h -> protocol version incremented

DeckerSU

Instance Creation of NotaryChecker:
You shouldn't create an instance of the NotaryChecker class in the header file (src/komodo_defs.h), as this will lead to multiple definition errors during compilation, like:

/usr/bin/ld: libbitcoin_util.a(libbitcoin_util_a-util.o):/media/decker/data1tb/marmara/src/komodo_defs.h:742: multiple definition of `notaryChecker'; marmarad-bitcoind.o:/media/decker/data1tb/marmara/src/komodo_defs.h:742: first defined here
/usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-rpcwallet.o):/media/decker/data1tb/marmara/src/./cc/../komodo_defs.h:742: multiple definition of `notaryChecker'; marmarad-bitcoind.o:/media/decker/data1tb/marmara/src/komodo_defs.h:742: first defined here

To resolve this, you can comment out the object creation like so:

// NotaryChecker notaryChecker;

and move it to another appropriate place in the codebase.

Incorrect Notary Pubkeys Array:
The notary pubkeys array you're using is incorrect. Since Marmara (MCL) is a third-party coin from the dPoW perspective, you need to grab the correct pubkeys from here. Without this, dPoW for MCL will not function properly.
S8 Activation for MCL (Timestamp-based):
For MCL, as a third-party coin in Komodo's dPoW ecosystem, the S8 activation is timestamp-based. Thus, having nS8Timestamp = 1728049053 is sufficient. You do not need to calculate the block height on the MCL chain to match Fri, Oct 4, 2024. So, the answer to "Should nS8HardforkHeight be calculated according to Marmara chain blocks?" is no, it shouldn’t.
NotaryChecker Class Usage:
As for the question "Should the NotaryChecker class be added?", the answer is up to you. The primary purpose of this class is to perform a runtime check on the pubkeys array. If anything is incorrect or missing, the daemon won't start. If you want to implement this kind of protection against mistakes (like typos) in MCL, you may want to include the class in your codebase.

…latform/dPoW/blob/5198f6d8edf65bf93ed46bfd69e37a00ee202885/iguana/3rd_party - removed NotaryChecker

DeckerSU

LGTM.

p.s. JFYI, the Windows build will fail if ./src/cc/customcc.so exists / remains from prev. builds:

make: *** No rule to make target 'libcc.dll', needed by 'marmarad.exe'.  Stop.

p.p.s. One more note: It’s better to use your own DNS seeder rather than fixed seed nodes. This way, it will be easier to update A-records if the seed nodes change, rather than modifying hardcoded seed nodes.

dimxy · 2024-09-19T16:52:10Z

changing the default indexes (enabling addressindex and spendindex by default) will likely cause all nodes to resync or reindex after the update

addressindex and spendindex as true are critical for marmara to work: some cc functions require them. Those settings are enabled from the chain start

DeckerSU · 2024-09-19T16:58:41Z

addressindex and spendindex as true are critical for marmara to work: some cc functions require them. Those settings are enabled from the chain start

You're absolutely right—I just mixed up the launch parameters with another chain. In this case, -addressindex=1 -spentindex=1 has been there from the beginning. I've deleted my initial comment.

dimxy · 2024-09-19T18:56:49Z

btw I managed to build marmara.exe (from fresh repo though) on ubuntu 18.04 and 20.04 (libcc.dll present in the src dir too)

DeckerSU · 2024-09-19T20:00:06Z

btw I managed to build marmara.exe (from fresh repo though) on ubuntu 18.04 and 20.04

Ubuntu 22.04.5 LTS, (fresh repo clone, developement branch)

Steps to reproduce:

./zcutil/build.sh -j8 - Ok.
./zcutil/build-win.sh -j8 - Fail with:

make: *** No rule to make target 'libcc.dll', needed by 'marmarad.exe'.  Stop.

And in the logs after (6) we can see:

+ echo Making cclib...
Making cclib...
+ ./makecustom

Linux
make: Nothing to be done for 'all'.
CUSTOMCC BUILD SUCCESSFUL
+ cd ./priceslibs
+ echo Making prices feeds custom libs...
Making prices feeds custom libs...
+ CC='x86_64-w64-mingw32-gcc-posix -g '
+ CXX='x86_64-w64-mingw32-g++-posix -g '
+ make dll
make: Nothing to be done for 'dll'.

So, the reason is make reports Nothing to be done for 'all' and even didn't try to build .dll.

p.s. Confirmed. The Windows build fails if ./src/cc/customcc.so exists. The following will work:

rm ./src/cc/customcc.so
./zcutil/build-win.sh -j8

However, the following will not:

touch ./src/cc/customcc.so
./zcutil/build-win.sh -j8

So, if customcc.so remains from previous builds, the Windows binaries will not be built successfully.

p.p.s. And actually, libcc.dll is not a PE32+ executable (DLL) (console) x86-64, for MS Windows. It's an Intel amd64 COFF object file, meaning it's a Linux binary. It’s linked with the resulting binary through this line: marmarad_LDADD += libcc.dll $(LIBSECP256K1). If you check the import sections of the resulting PE executable, you’ll see that it has no dependencies on libcc.dll, meaning it’s being statically linked anyway. 😉

rumeysayilmaz · 2024-09-20T07:43:00Z

p.p.s. One more note: It’s better to use your own DNS seeder rather than fixed seed nodes. This way, it will be easier to update A-records if the seed nodes change, rather than modifying hardcoded seed nodes.

@DeckerSU How can we do so? Could you explain how this approach can be implemented in the codebase?

Btw, we were unable to get a successful macOS build for marmara due to gcc. We would like to create another PR for this. We have noticed the following PRs in the komodo release.

How can we adopt these for marmara? We would appreciate your guidance.

dimxy · 2024-09-20T09:58:50Z

you’ll see that it has no dependencies on libcc.dll, meaning it’s being statically linked anyway.

It's built with -c flag. So it is named incorrectly but I guess still needed to build executable

DeckerSU · 2024-09-20T14:15:00Z

@DeckerSU How can we do so? Could you explain how this approach can be implemented in the codebase?

Adding a DNS seeder to the Marmara codebase is relatively easy to do. First, add your DNS seeder in sources here:

marmara/src/chainparams.cpp

Line 185 in 69277d1

    
           // vSeeds.push_back(CDNSSeedData("komodoseeds.com", "kmd.komodoseeds.com")); // Static contolled seeds list (Kolo)

For example:

vSeeds.push_back(CDNSSeedData("Marmara Super DNS Seeder", "mcl.komodoseeds.com"));

Then, create A-records for mcl.komodoseeds.com that point to your seed nodes. For example, mcl.komodoseeds.com already contains the IPv4 addresses of the MCL seed nodes. Here's what you'll see if you check it:

dig mcl.komodoseeds.org @1.1.1.1

; <<>> DiG 9.16.50-Debian <<>> mcl.komodoseeds.org @1.1.1.1
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 18827
;; flags: qr rd ra; QUERY: 1, ANSWER: 3, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 1232
;; QUESTION SECTION:
;mcl.komodoseeds.org.		IN	A

;; ANSWER SECTION:
mcl.komodoseeds.org.	300	IN	A	5.189.149.242
mcl.komodoseeds.org.	300	IN	A	161.97.146.150
mcl.komodoseeds.org.	300	IN	A	149.202.158.145

;; Query time: 12 msec
;; SERVER: 1.1.1.1#53(1.1.1.1)
;; WHEN: Fri Sep 20 14:06:32 UTC 2024
;; MSG SIZE  rcvd: 96

Do the same for *.marmara.io by creating A-records that point to your seed nodes. Then, include these updated DNS seeders in the Marmara codebase.

DeckerSU · 2024-09-20T14:19:12Z

Btw, we were unable to get a successful macOS build for marmara due to gcc.

As you mentioned above, the Komodo codebase uses Clang as the native macOS compiler. Therefore, you can follow the same approach. To achieve this, cherry-pick the necessary commits from the Komodo pull request: KomodoPlatform#618.

DeckerSU · 2024-09-20T14:26:21Z

It's built with -c flag. So it is named incorrectly but I guess still needed to build executable

Yes, of course, the custom CC library is necessary to build. However, there's no need to explicitly reference its .dll since it’s not a dynamically linked library. Instead, it's included as a static dependency in the binaries. We should build the static libcc.a library, as it’s done in the upstream Komodo codebase. The current method of building CCLib not only results in the errors mentioned but also causes confusion.

- marmarachain/marmara#47 - marmarachain/marmara@20a8fde - https://github.com/KomodoPlatform/dPoW/blob/5198f6d8edf65bf93ed46bfd69e37a00ee202885/iguana/3rd_party

rumeysayilmaz requested review from dimxy and DeckerSU September 18, 2024 08:01

DeckerSU requested changes Sep 18, 2024

View reviewed changes

- corrected pubkeys for S8 NN nodes as per https://github.com/KomodoP…

20a8fde

…latform/dPoW/blob/5198f6d8edf65bf93ed46bfd69e37a00ee202885/iguana/3rd_party - removed NotaryChecker

rumeysayilmaz requested a review from DeckerSU September 19, 2024 08:44

DeckerSU approved these changes Sep 19, 2024

View reviewed changes

update deprecated workflow actions version

c6a08b7

rumeysayilmaz merged commit 8916a20 into development Sep 20, 2024
8 of 9 checks passed

rumeysayilmaz mentioned this pull request Sep 25, 2024

macOS build for marmara and remove explicit reference to libcc.dl and build it as in komodo codebase #49

Open

rumeysayilmaz mentioned this pull request Oct 9, 2024

added DNS Seeder in Marmara codebase #53

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

S8 NN Update preparations #47

S8 NN Update preparations #47

rumeysayilmaz commented Sep 18, 2024

DeckerSU left a comment

DeckerSU left a comment •

edited

Loading

dimxy commented Sep 19, 2024

DeckerSU commented Sep 19, 2024

dimxy commented Sep 19, 2024 •

edited

Loading

DeckerSU commented Sep 19, 2024 •

edited

Loading

rumeysayilmaz commented Sep 20, 2024

dimxy commented Sep 20, 2024

DeckerSU commented Sep 20, 2024

DeckerSU commented Sep 20, 2024

DeckerSU commented Sep 20, 2024 •

edited

Loading

S8 NN Update preparations #47

S8 NN Update preparations #47

Conversation

rumeysayilmaz commented Sep 18, 2024

DeckerSU left a comment

Choose a reason for hiding this comment

DeckerSU left a comment • edited Loading

Choose a reason for hiding this comment

dimxy commented Sep 19, 2024

DeckerSU commented Sep 19, 2024

dimxy commented Sep 19, 2024 • edited Loading

DeckerSU commented Sep 19, 2024 • edited Loading

rumeysayilmaz commented Sep 20, 2024

dimxy commented Sep 20, 2024

DeckerSU commented Sep 20, 2024

DeckerSU commented Sep 20, 2024

DeckerSU commented Sep 20, 2024 • edited Loading

DeckerSU left a comment •

edited

Loading

dimxy commented Sep 19, 2024 •

edited

Loading

DeckerSU commented Sep 19, 2024 •

edited

Loading

DeckerSU commented Sep 20, 2024 •

edited

Loading