Skip to content

Split the part that read the blockundo data from disk so the actual u… #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
deadalnix merged 1 commit into from
May 12, 2017
Merged

Split the part that read the blockundo data from disk so the actual u… #8

deadalnix merged 1 commit into from
May 12, 2017
+163 −28

Conversation

@deadalnix
Copy link
Member

@deadalnix deadalnix commented May 11, 2017

The first read the undo data from disk and the second does the undo. This ensure we don't depend on the state of the filesystem to undo and makes is easier to unit test. The patch also includes a basic unit test for undo.

@deadalnix deadalnix force-pushed the testundo branch 3 times, most recently from d133fe8 to 3c4d18d Compare May 12, 2017 09:49
ftrader
ftrader suggested changes May 12, 2017
View reviewed changes
Copy link
Contributor

@ftrader ftrader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing file header (copyright) updates to undo.h, validation.{h,cpp}

See other inline comments

src/validation.cpp Outdated
}
// Restore inputs.
if (i < 1) {
// Skipe the coinbase.
Copy link
Contributor

@ftrader ftrader May 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spelling error, should be 'Skip'

src/validation.cpp Outdated

// undo transactions in reverse order
// Undo transactions in reverse order.
for (int i = block.vtx.size() - 1; i >= 0; i--) {
Copy link
Contributor

@ftrader ftrader May 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would consider adding a sanity check that block.vtx.size() > 0

Copy link
Member Author

@deadalnix deadalnix May 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It must because of the previous check (and no, blockUndo.vtxundo.size() cannot be size_t.max, there wouldn't be enough addressable memory for this to happen).

Copy link
Member Author

@deadalnix deadalnix May 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That being said I think I can rewrite this in a way that do have this edge case.

src/validation.cpp
bool *pfClean) {
assert(pindex->GetBlockHash() == view.GetBestBlock());

if (pfClean) *pfClean = false;
Copy link
Contributor

@ftrader ftrader May 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove as pfClean is passed through to ApplyBlockUndo() and is initialized there if needed

Copy link
Member Author

@deadalnix deadalnix May 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't reach ApplyBlockUndo, we still need to set the flag as expected by the caller.

Copy link
Contributor

@ftrader ftrader May 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, I didn't consider that.

@deadalnix
Copy link
Member Author

deadalnix commented May 12, 2017

Ok I added the copy right blurb, rewrote the loop so it doesn't have edge case when the size is 0 and fixed typo. I think we should keep setting pfClean as this matter is case there is an error before we reach ApplyBlockUndo.

@ftrader
Copy link
Contributor

ftrader commented May 12, 2017 •
edited
Loading

Please fix undo.h / validation.{h,cpp} copyright year:

// Copyright (c) 20017 The Bitcoin developers

can be in next update

@deadalnix
Copy link
Member Author

deadalnix commented May 12, 2017

Done.

@deadalnix
Split the block undoing in two parts and add unit tests.
f8001c0 
The first read the undo data from disk and the second does the undo. This ensure we don't depend on the state of the filesystem to undo and makes is easier to unit test. The patch also includes a basic unit test for undo.
@ftrader
Copy link
Contributor

ftrader commented May 12, 2017

All good from my side.

@deadalnix deadalnix merged commit 880d847 into Bitcoin-ABC:master May 12, 2017
@deadalnix deadalnix deleted the testundo branch May 12, 2017 22:07
lateminer pushed a commit to lateminer/bitcoin-abc that referenced this pull request Apr 1, 2018
@janko33bd
Merge pull request Bitcoin-ABC#8 from lateminer/dev
3fb35c5 
Fix errors, part 4, add address indexes
Mengerian pushed a commit to Mengerian/bitcoin-abc that referenced this pull request Oct 31, 2019
@jasonbcox @schancel
Bump version on homepage (Bitcoin-ABC#8)
e089f9d
deadalnix pushed a commit that referenced this pull request Dec 28, 2020
@majcosta
[backport#17674] tests: Add initialization order fiasco detection in …
929b492 
…Travis

Summary:
1f9d5af4f197e7cc0469a0bb25dcbc51dfa537f4 tests: Add initialization order fiasco detection in Travis (practicalswift)

Pull request description:

  Add initialization order fiasco detection in Travis :)

  Context: bitcoin/bitcoin#17670 (comment)

  This would have caught the `events_hasher` initialization order issue introduced in #17573  and fixed in #17670.

  Output in case of an initialization order fiasco:

  ```
  ==7934==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x557098d79200 at pc 0x55709796b9a3 bp 0x7ffde524dc30 sp 0x7ffde524dc28
  READ of size 8 at 0x557098d79200 thread T0
      #0 0x55709796b9a2 in CSHA256::Finalize(unsigned char*) src/crypto/sha256.cpp:667:25
      #1 0x5570978150e9 in SeedEvents(CSHA512&) src/random.cpp:462:19
      #2 0x5570978145e1 in SeedSlow(CSHA512&) src/random.cpp:482:5
      #3 0x5570978149a3 in SeedStartup(CSHA512&, (anonymous namespace)::RNGState&) src/random.cpp:527:5
      #4 0x55709781102d in ProcRand(unsigned char*, int, RNGLevel) src/random.cpp:571:9
      #5 0x557097810d19 in GetRandBytes(unsigned char*, int) src/random.cpp:576:59
      #6 0x557096c2f9d5 in (anonymous namespace)::CSignatureCache::CSignatureCache() src/script/sigcache.cpp:34:9
      #7 0x557096511977 in __cxx_global_var_init.7 src/script/sigcache.cpp:67:24
      #8 0x5570965119f8 in _GLOBAL__sub_I_sigcache.cpp src/script/sigcache.cpp
      #9 0x557097bba4ac in __libc_csu_init (src/bitcoind+0x18554ac)
      #10 0x7f214b1c2b27 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:266
      #11 0x5570965347d9 in _start (src/bitcoind+0x1cf7d9)

  0x557098d79200 is located 96 bytes inside of global variable 'events_hasher' defined in 'random.cpp:456:16' (0x557098d791a0) of size 104
    registered at:
      #0 0x557096545dfd in __asan_register_globals compiler-rt/lib/asan/asan_globals.cpp:360:3
      #1 0x557097817f8b in asan.module_ctor (src/bitcoind+0x14b2f8b)

  SUMMARY: AddressSanitizer: initialization-order-fiasco src/crypto/sha256.cpp:667:25 in CSHA256::Finalize(unsigned char*)
  ```

---

Backport of Core [[bitcoin/bitcoin#17674 | PR17674]]

Depends on D8756

Test Plan:
  cmake -GNinja -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DENABLE_SANITIZERS=address
  ninja all check check-functional

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D8757
imaginaryusername pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request Jul 10, 2024
@sipa @cculianu
[backport] Delete limitedmap as it is unused now
d9e0b6f 
This is commit Bitcoin-ABC#8 from:  bitcoin/bitcoin#19988
johann-sys added a commit to johann-sys/bitcoin-abc that referenced this pull request May 15, 2025
@johann-sys
Update COPYING
21b6864 
logoMainnet Explorer
learn more about eCash !
block height/hash, txid, address
Address
ecash:qz60j37wmr0wv2k9ksz5e4qkjdzgxggg3sr98q45ug
Details
JSON
Summary
Script Public Key
76a914b4f947ced8dee62ac5b4054cd41693448321088c88ac
Balance
0
Transactions
2
QR Code
ecash:qz60j37wmr0wv2k9ksz5e4qkjdzgxggg3sr98q45ug
Scripthash
6b7253cd13966210f51a0482486181fb036f639aa1f2fbf6ddd0daee039d4450
Is Valid?
Is Script?
2 Transactions
Newest First
Bitcoin-ABC#2–
0357c59fffffd39cd438db127a39e88887ab818b7b9933db03ba7e44dc73a98f
-552,510.39XEC
6/2, 2014 19:24
(10y, 11mo, 13d ago)
> Input #0 / 0
p2pkhe4a57a901c8d13e411ad47024… #0
(addr:ecash:qz60j37wmr0wv2k9ksz5e4qkjdzgxggg3sr98q45ug
552,510.39XEC
Total Input:
552,510.39XEC
< Output #0 / 1
p2pkh
ecash:qzgyfwdkwn9zx6h9vvl0tafupvk2drkdtg0kzut2al
204,747.39XEC
< Output Bitcoin-ABC#1 / 1
p2pkh
ecash:qzq4es9v7eq2wfm6rxwhpazaryes4xyedsfpjlylcd
347,763XEC
Total Output:
552,510.39XEC
Bitcoin-ABC#1–
e4a57a901c8d13e411ad47024dcba3e04f268c99c6fc1e99f0bf14afc2e81c7f
+552,510.39XEC
5/19, 2014 19:36
(10y, 11mo, 27d ago)
> Input #0 / 18
p2pkh01f5cb95681c922d44ae517da… Bitcoin-ABC#1
(addr:ecash:qqc24zdvcnjhnusda0jkvrrgmkg0jnl88vg4txjsg5
0.01XEC
> Input Bitcoin-ABC#1 / 18
p2pkh6e8f883eb799a9a286fe492a3… #0
(addr:ecash:qr0tjhmsrgjsf3k72xdtqv7flxq750x645z0wwxpmq
40.47XEC
> Input Bitcoin-ABC#2 / 18
p2pkh901bde191beb61d2fc5bf480c… Bitcoin-ABC#1
(addr:ecash:qrjckd8ep6grzchnsv9gcdzwcpwqjazn6unlutuff9
0.01XEC
> Input Bitcoin-ABC#3 / 18
p2pkh8cab265e1896f173757325295… #0
(addr:ecash:qqh24fuuud0yp5pn2q4spdh8yfwd8kskzyhtmgu8c6
53,927.28XEC
> Input Bitcoin-ABC#4 / 18
p2pkhbb79adc9ad5e696ed8fe08f74… Bitcoin-ABC#1
(addr:ecash:qqe3egddd4s95glp0kczh02evv3q326pvq9r25j3jn
0.01XEC
> Input Bitcoin-ABC#5 / 18
p2pkh4dacaefb3d43daed074189358… Bitcoin-ABC#1
(addr:ecash:qrrdyutsjnr5xy7fd60dgra9aw8jc5gjdy00exz55l
0.01XEC
> Input Bitcoin-ABC#6 / 18
p2pkh65f5d4d7b80e0cd4d4d477b65… Bitcoin-ABC#1
(addr:ecash:qrgs5rvlxee0f4cayf9tcslzdn3h7kky95jvyljnqq
0.01XEC
> Input Bitcoin-ABC#7 / 18
p2pkhf183d53245ab3a17a3278e696… #0
(addr:ecash:qrjckd8ep6grzchnsv9gcdzwcpwqjazn6unlutuff9
9,438XEC
> Input Bitcoin-ABC#8 / 18
p2pkh7f6ce8e4e912b8da7da9f43ac… Bitcoin-ABC#1
(addr:ecash:qzzy34srdrgtnzqcy3adgcxh278x78mwmuya540hkx
0.01XEC
> Input Bitcoin-ABC#9 / 18
p2pkh3922dd199e8901bc7dffc06fe… #0
(addr:ecash:qpycut36dx0j03us0c4l3hn9cfhyclu9e5nxxhz3dg
0.01XEC
> Input Bitcoin-ABC#10 / 18
p2pkhd00af226755d2f6f78c3c213d… Bitcoin-ABC#1
(addr:ecash:qpnyhed8vzp34mtg63cxxy6cdjcsdcnazsff2sylqt
0.01XEC
> Input Bitcoin-ABC#11 / 18
p2pkha04e516237bc80d3716a174e0… #0
(addr:ecash:qptuamelkkux8r5uuv8uqlzmn0ag2542fydyslz7s5
399,970XEC
> Input Bitcoin-ABC#12 / 18
p2pkhd63ccab1d9b1fe1788d21d38b… Bitcoin-ABC#1
(addr:ecash:qryd3ex3zq860sqzney5guqlf5r2y8huzcy0ng8uuv
5.13XEC
> Input Bitcoin-ABC#13 / 18
p2pkh0fe15b8cc74cfd189ca866132… Bitcoin-ABC#16
(addr:ecash:qrjckd8ep6grzchnsv9gcdzwcpwqjazn6unlutuff9
80XEC
> Input Bitcoin-ABC#14 / 18
p2pkh679fde36f4059292df243e074… Bitcoin-ABC#1
(addr:ecash:qzuch5gavmtnl7ql53xhswquhu8jz6p7z5jfej3nam
0.01XEC
> Input Bitcoin-ABC#15 / 18
p2pkha2a641d13a94a3dcc77cacaa8… Bitcoin-ABC#6
(addr:ecash:qrjckd8ep6grzchnsv9gcdzwcpwqjazn6unlutuff9
400XEC
> Input Bitcoin-ABC#16 / 18
p2pkh46f3140ef52750b7c380e8f91… Bitcoin-ABC#200
(addr:ecash:qrjckd8ep6grzchnsv9gcdzwcpwqjazn6unlutuff9
20.68XEC
> Input Bitcoin-ABC#17 / 18
p2pkhafa535d4b4e2345cf7fce38f9… Bitcoin-ABC#46
(addr:ecash:qrjckd8ep6grzchnsv9gcdzwcpwqjazn6unlutuff9
96.45XEC
> Input Bitcoin-ABC#18 / 18
p2pkh6ab5f652b42b27703f5c1fc34… Bitcoin-ABC#1
(addr:ecash:qzrn4d90fw6029pmdn95r5wy68z4638luyv5rnl303
88,932.29XEC
Total Input:
552,910.39XEC
< Output #0 / 0
p2pkh
ecash:qz60j37wmr0wv2k9ksz5e4qkjdzgxggg3sr98q45ug
552,510.39XEC
Total Output:
552,510.39XEC
hosted by bitcoinabc.org
Fabcien pushed a commit that referenced this pull request Oct 31, 2025
@BytesOfMan
[ecash-agora] Pin the dependencies with updated dockerfile
7c0f2be 
Summary:
Yet another iteration on this but this time confirmed with local build and debug logs.

We can still pin these versions, so we should.

imo we should leave these debug logs in case there are future issues with CI, e.g. could happen if a diff lands that updates 2 modules at once, may need to see what dep got in.

Test Plan:
```
docker build --progress=plain -f ecash-agora.Dockerfile -t ecash-agora_local .
```

Output (confirms we get the latest versions and not local versions)

```
#0 building with "default" instance using docker driver

#1 [internal] load build definition from ecash-agora.Dockerfile
#1 transferring dockerfile: 1.43kB done
#1 DONE 0.0s

#2 [internal] load metadata for docker.io/library/node:22-bookworm-slim
#2 DONE 2.7s

#3 [internal] load .dockerignore
#3 transferring context: 2B done
#3 DONE 0.0s

#4 [1/5] FROM docker.io/library/node:22-bookworm-slim@sha256:7e3a5fac109e00e9f2616cd24f5227e2eb0d473bf33b98d661b0c1862f6bfbbd
#4 DONE 0.0s

#5 [internal] load build context
#5 transferring context: 951.95kB 0.1s done
#5 DONE 0.1s

#6 [2/5] WORKDIR /app/modules/ecash-agora
#6 CACHED

#7 [3/5] COPY modules/ecash-agora .
#7 DONE 0.4s

#8 [4/5] RUN ECLIB_VERSION=$(npm view ecash-lib version)     && CCLIENT_VERSION=$(npm view chronik-client version)     && EWALLET_VERSION=$(npm view ecash-wallet version)     && echo "Latest versions from npm:"     && echo "  ecash-lib: $ECLIB_VERSION"     && echo "  chronik-client: $CCLIENT_VERSION"     && echo "  ecash-wallet: $EWALLET_VERSION"     && npm pkg set dependencies.ecash-lib=$ECLIB_VERSION     && npm pkg set dependencies.chronik-client=$CCLIENT_VERSION     && npm pkg set dependencies.ecash-wallet=$EWALLET_VERSION     && echo "Updated package.json dependencies:"     && cat package.json | grep -A 5 '"dependencies"'
#8 4.790 Latest versions from npm:
#8 4.790   ecash-lib: 4.5.1
#8 4.790   chronik-client: 3.4.1
#8 4.790   ecash-wallet: 2.2.0
#8 4.971 Updated package.json dependencies:
#8 4.972     "dependencies": {
#8 4.972         "chronik-client": "3.4.1",
#8 4.972         "ecash-lib": "4.5.1",
#8 4.972         "ecash-wallet": "2.2.0"
#8 4.972     }
#8 4.972 }
#8 DONE 5.0s

#9 [5/5] RUN rm -f package-lock.json     && npm install     && npm run build
#9 7.044
#9 7.044 added 41 packages, changed 3 packages, and audited 315 packages in 7s
#9 7.044
#9 7.044 43 packages are looking for funding
#9 7.044   run `npm fund` for details
#9 7.047
#9 7.047 7 vulnerabilities (1 low, 5 moderate, 1 high)
#9 7.047
#9 7.047 To address all issues, run:
#9 7.047   npm audit fix
#9 7.047
#9 7.047 Run `npm audit` for details.
#9 7.136
#9 7.136 > ecash-agora@2.6.3 build
#9 7.136 > tsc && tsc -p ./tsconfig.build.json
#9 7.136
#9 DONE 10.2s

#10 exporting to image
#10 exporting layers
#10 exporting layers 0.3s done
#10 writing image sha256:dcca77ce4b5f49de525a11894ad54cdac51e898e91af481a16b69a7c119abdd2 done
#10 naming to docker.io/library/ecash-agora_local done
#10 DONE 0.3s
```

Output:

```
docker run --rm ecash-agora_local cat /app/modules/ecash-agora/package.json
{
    "name": "ecash-agora",
    "version": "2.6.2",
    "description": "Library for interacting with the eCash Agora protocol",
    "main": "./dist/index.js",
    "scripts": {
        "build": "tsc && tsc -p ./tsconfig.build.json",
        "test": "mocha --import=tsx ./src/*.test.ts ./src/**/*.test.ts",
        "integration-tests": "mocha --import=tsx ./tests/*.test.ts --timeout 60000",
        "coverage": "nyc npm run test",
        "junit": "npm run test --reporter mocha-junit-reporter"
    },
    "repository": {
        "type": "git",
        "url": "git+https://github.com/Bitcoin-ABC/bitcoin-abc.git",
        "directory": "modules/ecash-agora"
    },
    "keywords": [
        "ecash",
        "xec",
        "bitcoin"
    ],
    "author": "Bitcoin ABC",
    "license": "MIT",
    "bugs": {
        "url": "https://github.com/Bitcoin-ABC/bitcoin-abc/issues"
    },
    "homepage": "https://github.com/Bitcoin-ABC/bitcoin-abc#readme",
    "devDependencies": {
        "@istanbuljs/nyc-config-typescript": "^1.0.2",
        "@types/chai": "^4.3.14",
        "@types/chai-as-promised": "^7.1.8",
        "@types/mocha": "^10.0.6",
        "@types/node": "^20.12.7",
        "chai": "^4.4.1",
        "chai-as-promised": "^7.1.1",
        "mocha": "^10.4.0",
        "mocha-junit-reporter": "^2.2.1",
        "nyc": "^15.1.0",
        "source-map-support": "^0.5.21",
        "ts-node": "^10.9.2",
        "tsx": "^4.7.2",
        "typescript": "^5.4.3"
    },
    "dependencies": {
        "chronik-client": "3.4.1",
        "ecash-lib": "4.5.1",
        "ecash-wallet": "2.2.0"
    }
}

```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D18857
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ftrader ftrader ftrader approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@deadalnix @ftrader