Skip to content
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.

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

feat: switch to raw multihashes for blocks #6816

Merged
merged 11 commits into from
Dec 13, 2021
2 changes: 1 addition & 1 deletion core/commands/refs.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ var RefsLocalCmd = &cmds.Command{
Helptext: cmds.HelpText{
Tagline: "List all local references.",
ShortDescription: `
Displays the hashes of all local objects.
Displays the hashes of all local objects. NOTE: This treats all local objects as "raw blocks" and returns CIDv1-Raw CIDs.
`,
},

Expand Down
2 changes: 0 additions & 2 deletions core/node/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/ipfs/go-filestore"
"github.com/ipfs/go-ipfs/core/node/helpers"
"github.com/ipfs/go-ipfs/repo"
"github.com/ipfs/go-ipfs/thirdparty/cidv0v1"
"github.com/ipfs/go-ipfs/thirdparty/verifbs"
)

Expand Down Expand Up @@ -41,7 +40,6 @@ func BaseBlockstoreCtor(cacheOpts blockstore.CacheOpts, nilRepo bool, hashOnRead
}

bs = blockstore.NewIdStore(bs)
bs = cidv0v1.NewBlockstore(bs)

if hashOnRead { // TODO: review: this is how it was done originally, is there a reason we can't just pass this directly?
bs.HashOnRead(true)
Expand Down
37 changes: 35 additions & 2 deletions gc/gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ type Result struct {
Error error
}

// converts a set of CIDs with different codecs to a set of CIDs with the raw codec.
func toRawCids(set *cid.Set) (*cid.Set, error) {
newSet := cid.NewSet()
err := set.ForEach(func(c cid.Cid) error {
newSet.Add(cid.NewCidV1(cid.Raw, c.Hash()))
return nil
})
return newSet, err
}

// GC performs a mark and sweep garbage collection of the blocks in the blockstore
// first, it creates a 'marked' set and adds to it the following:
// - all recursively pinned blocks, plus all of their descendants (recursively)
Expand Down Expand Up @@ -60,6 +70,17 @@ func GC(ctx context.Context, bs bstore.GCBlockstore, dstor dstore.Datastore, pn
}
return
}

// The blockstore reports raw blocks. We need to remove the codecs from the CIDs.
gcs, err = toRawCids(gcs)
aschmahmann marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
select {
case output <- Result{Error: err}:
case <-ctx.Done():
}
return
}

keychan, err := bs.AllKeysChan(ctx)
if err != nil {
select {
Expand All @@ -79,6 +100,8 @@ func GC(ctx context.Context, bs bstore.GCBlockstore, dstor dstore.Datastore, pn
if !ok {
break loop
}
// NOTE: assumes that all CIDs returned by the keychan are _raw_ CIDv1 CIDs.
// This means we keep the block as long as we want it somewhere (CIDv1, CIDv0, Raw, other...).
if !gcs.Has(k) {
err := bs.DeleteBlock(ctx, k)
removed++
Expand Down Expand Up @@ -154,7 +177,9 @@ func Descendants(ctx context.Context, getLinks dag.GetLinks, set *cid.Set, roots

for _, c := range roots {
// Walk recursively walks the dag and adds the keys to the given set
err := dag.Walk(ctx, verifyGetLinks, c, set.Visit, dag.Concurrent())
err := dag.Walk(ctx, verifyGetLinks, c, func(k cid.Cid) bool {
return set.Visit(toCidV1(k))
}, dag.Concurrent())

if err != nil {
err = verboseCidError(err)
Expand All @@ -165,6 +190,14 @@ func Descendants(ctx context.Context, getLinks dag.GetLinks, set *cid.Set, roots
return nil
}

// toCidV1 converts any CIDv0s to CIDv1s.
func toCidV1(c cid.Cid) cid.Cid {
if c.Version() == 0 {
return cid.NewCidV1(c.Type(), c.Hash())
}
return c
}

// ColoredSet computes the set of nodes in the graph that are pinned by the
// pins in the given pinner.
func ColoredSet(ctx context.Context, pn pin.Pinner, ng ipld.NodeGetter, bestEffortRoots []cid.Cid, output chan<- Result) (*cid.Set, error) {
Expand Down Expand Up @@ -225,7 +258,7 @@ func ColoredSet(ctx context.Context, pn pin.Pinner, ng ipld.NodeGetter, bestEffo
return nil, err
}
for _, k := range dkeys {
gcs.Add(k)
gcs.Add(toCidV1(k))
}

ikeys, err := pn.InternalPins(ctx)
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ require (
github.com/ipfs/go-ds-leveldb v0.5.0
github.com/ipfs/go-ds-measure v0.2.0
github.com/ipfs/go-fetcher v1.6.1
github.com/ipfs/go-filestore v0.1.0
github.com/ipfs/go-filestore v1.1.0
github.com/ipfs/go-fs-lock v0.0.7
github.com/ipfs/go-graphsync v0.11.0
github.com/ipfs/go-ipfs-blockstore v0.2.1
github.com/ipfs/go-ipfs-blockstore v1.1.1
github.com/ipfs/go-ipfs-chunker v0.0.5
github.com/ipfs/go-ipfs-cmds v0.6.0
github.com/ipfs/go-ipfs-config v0.18.0
Expand Down
10 changes: 6 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -454,8 +454,8 @@ github.com/ipfs/go-ds-measure v0.2.0/go.mod h1:SEUD/rE2PwRa4IQEC5FuNAmjJCyYObZr9
github.com/ipfs/go-fetcher v1.5.0/go.mod h1:5pDZ0393oRF/fHiLmtFZtpMNBQfHOYNPtryWedVuSWE=
github.com/ipfs/go-fetcher v1.6.1 h1:UFuRVYX5AIllTiRhi5uK/iZkfhSpBCGX7L70nSZEmK8=
github.com/ipfs/go-fetcher v1.6.1/go.mod h1:27d/xMV8bodjVs9pugh/RCjjK2OZ68UgAMspMdingNo=
github.com/ipfs/go-filestore v0.1.0 h1:qxvDVTzGrbQElddMmkwsJwJn+fDwWb3pHQHtKc1H0a8=
github.com/ipfs/go-filestore v0.1.0/go.mod h1:0KTrzoJnJ3sJDEDM09Vq8nz8H475rRyeq4i0n/bpF00=
github.com/ipfs/go-filestore v1.1.0 h1:Pu4tLBi1bucu6/HU9llaOmb9yLFk/sgP+pW764zNDoE=
github.com/ipfs/go-filestore v1.1.0/go.mod h1:6e1/5Y6NvLuCRdmda/KA4GUhXJQ3Uat6vcWm2DJfxc8=
github.com/ipfs/go-fs-lock v0.0.7 h1:6BR3dajORFrFTkb5EpCUFIAypsoxpGpDSVUdFwzgL9U=
github.com/ipfs/go-fs-lock v0.0.7/go.mod h1:Js8ka+FNYmgQRLrRXzU3CB/+Csr1BwrRilEcvYrHhhc=
github.com/ipfs/go-graphsync v0.11.0 h1:PiiD5CnoC3xEHMW8d6uBGqGcoTwiMB5d9CORIEyF6iA=
Expand All @@ -464,8 +464,9 @@ github.com/ipfs/go-ipfs-blockstore v0.0.1/go.mod h1:d3WClOmRQKFnJ0Jz/jj/zmksX0ma
github.com/ipfs/go-ipfs-blockstore v0.1.0/go.mod h1:5aD0AvHPi7mZc6Ci1WCAhiBQu2IsfTduLl+422H6Rqw=
github.com/ipfs/go-ipfs-blockstore v0.1.4/go.mod h1:Jxm3XMVjh6R17WvxFEiyKBLUGr86HgIYJW/D/MwqeYQ=
github.com/ipfs/go-ipfs-blockstore v0.1.6/go.mod h1:Jxm3XMVjh6R17WvxFEiyKBLUGr86HgIYJW/D/MwqeYQ=
github.com/ipfs/go-ipfs-blockstore v0.2.1 h1:624eIDnkZWNdWbp/N8aDBOUtSY0YW75aJu+vbxnNlkA=
github.com/ipfs/go-ipfs-blockstore v0.2.1/go.mod h1:jGesd8EtCM3/zPgx+qr0/feTXGUeRai6adgwC+Q+JvE=
github.com/ipfs/go-ipfs-blockstore v1.1.1 h1:a4koS3l+Fzl43LAAn51/N+Yn4AjCX4AGvoZLqqkrD/g=
github.com/ipfs/go-ipfs-blockstore v1.1.1/go.mod h1:w51tNR9y5+QXB0wkNcHt4O2aSZjTdqaEWaQdSxEyUOY=
github.com/ipfs/go-ipfs-blocksutil v0.0.1 h1:Eh/H4pc1hsvhzsQoMEP3Bke/aW5P5rVM1IWFJMcGIPQ=
github.com/ipfs/go-ipfs-blocksutil v0.0.1/go.mod h1:Yq4M86uIOmxmGPUHv/uI7uKqZNtLb449gwKqXjIsnRk=
github.com/ipfs/go-ipfs-chunker v0.0.1/go.mod h1:tWewYK0we3+rMbOh7pPFGDyypCtvGcBFymgY4rSDLAw=
Expand All @@ -479,8 +480,9 @@ github.com/ipfs/go-ipfs-delay v0.0.0-20181109222059-70721b86a9a8/go.mod h1:8SP1Y
github.com/ipfs/go-ipfs-delay v0.0.1 h1:r/UXYyRcddO6thwOnhiznIAiSvxMECGgtv35Xs1IeRQ=
github.com/ipfs/go-ipfs-delay v0.0.1/go.mod h1:8SP1YXK1M1kXuc4KJZINY3TQQ03J2rwBG9QfXmbRPrw=
github.com/ipfs/go-ipfs-ds-help v0.0.1/go.mod h1:gtP9xRaZXqIQRh1HRpp595KbBEdgqWFxefeVKOV8sxo=
github.com/ipfs/go-ipfs-ds-help v0.1.1 h1:IW/bXGeaAZV2VH0Kuok+Ohva/zHkHmeLFBxC1k7mNPc=
github.com/ipfs/go-ipfs-ds-help v0.1.1/go.mod h1:SbBafGJuGsPI/QL3j9Fc5YPLeAu+SzOkI0gFwAg+mOs=
github.com/ipfs/go-ipfs-ds-help v1.1.0 h1:yLE2w9RAsl31LtfMt91tRZcrx+e61O5mDxFRR994w4Q=
github.com/ipfs/go-ipfs-ds-help v1.1.0/go.mod h1:YR5+6EaebOhfcqVCyqemItCLthrpVNot+rsOU/5IatU=
github.com/ipfs/go-ipfs-exchange-interface v0.0.1/go.mod h1:c8MwfHjtQjPoDyiy9cFquVtVHkO9b9Ob3FG91qJnWCM=
github.com/ipfs/go-ipfs-exchange-interface v0.1.0 h1:TiMekCrOGQuWYtZO3mf4YJXDIdNgnKWZ9IE3fGlnWfo=
github.com/ipfs/go-ipfs-exchange-interface v0.1.0/go.mod h1:ych7WPlyHqFvCi/uQI48zLZuAWVP5iTQPXEfVaw5WEI=
Expand Down
22 changes: 15 additions & 7 deletions test/sharness/t0080-repo.sh
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,25 @@ test_expect_success "remove direct pin" '
'

test_expect_success "'ipfs repo gc' removes file" '
ipfs repo gc >actual7 &&
grep "removed $HASH" actual7
ipfs block stat $HASH &&
ipfs repo gc &&
test_must_fail ipfs block stat $HASH
'

# Convert all to a base32-multihash as refs local outputs cidv1 raw
# Technically converting refs local output would suffice, but this is more
# future proof if we ever switch to adding the files with cid-version 1.
test_expect_success "'ipfs refs local' no longer shows file" '
EMPTY_DIR=QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn &&
ipfs refs local >actual8 &&
grep "QmYCvbfNbCwFR45HiNP45rwJgvatpiW38D961L5qAhUM5Y" actual8 &&
grep "$EMPTY_DIR" actual8 &&
grep "$HASH_WELCOME_DOCS" actual8 &&
test_must_fail grep "$HASH" actual8
HASH_MH=`cid-fmt -b base32 "%M" "$HASH"` &&
HARDCODED_HASH_MH=`cid-fmt -b base32 "%M" "QmYCvbfNbCwFR45HiNP45rwJgvatpiW38D961L5qAhUM5Y"` &&
EMPTY_DIR_MH=`cid-fmt -b base32 "%M" "$EMPTY_DIR"` &&
HASH_WELCOME_DOCS_MH=`cid-fmt -b base32 "%M" "$HASH_WELCOME_DOCS"` &&
ipfs refs local | cid-fmt -b base32 --filter "%M" >actual8 &&
Comment on lines +122 to +126
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need cid-fmt here, or can we use ipfs cid format? Generally, asking here and through the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know, this was done before ipfs cid format was there. Worth fixing in a different PR accross all tests?

grep "$HARDCODED_HASH_MH" actual8 &&
grep "$EMPTY_DIR_MH" actual8 &&
grep "$HASH_WELCOME_DOCS_MH" actual8 &&
test_must_fail grep "$HASH_MH" actual8
'

test_expect_success "adding multiblock random file succeeds" '
Expand Down
9 changes: 4 additions & 5 deletions test/sharness/t0081-repo-pinning.sh
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,10 @@ test_expect_success "pin lists look good" '
'

test_expect_success "'ipfs repo gc' succeeds" '
ipfs repo gc >gc_out_actual2 &&
echo "removed $HASH_FILE3" > gc_out_exp2 &&
echo "removed $HASH_FILE5" >> gc_out_exp2 &&
echo "removed $HASH_DIR3" >> gc_out_exp2 &&
test_includes_lines gc_out_exp2 gc_out_actual2
ipfs repo gc &&
test_must_fail ipfs block stat $HASH_FILE3 &&
test_must_fail ipfs block stat $HASH_FILE5 &&
test_must_fail ipfs block stat $HASH_DIR3
'

# use object links for HASH_DIR1 here because its children
Expand Down
5 changes: 3 additions & 2 deletions test/sharness/t0084-repo-read-rehash.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ test_check_bad_blocks() {
'

test_expect_success "block shows up in repo verify" '
test_expect_code 1 ipfs repo verify > verify_out &&
grep "$H_BLOCK2" verify_out
test_expect_code 1 ipfs repo verify | cid-fmt --filter -b base32 "%M" > verify_out &&
H_BLOCK2_MH=`cid-fmt -b base32 "%M" $H_BLOCK2` &&
grep "$H_BLOCK2_MH" verify_out
'
}

Expand Down
47 changes: 27 additions & 20 deletions test/sharness/t0087-repo-robust-gc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,35 @@ test_description="Test robustness of garbage collector"
. lib/test-lib.sh
set -e

to_raw_cid() {
ipfs cid format -b b --codec raw -v 1 "$1"
}

test_gc_robust_part1() {

test_expect_success "add a 1MB file with --raw-leaves" '
random 1048576 56 > afile &&
HASH1=`ipfs add --raw-leaves -q afile`
HASH1=`ipfs add --raw-leaves -q --cid-version 1 afile` &&
REFS=`ipfs refs -r $HASH1` &&
read LEAF1 LEAF2 LEAF3 LEAF4 < <(echo $REFS)
'

HASH1FILE=.ipfs/blocks/L3/CIQNIPL4GP62ZMNNSLZ2G33Z3T5VAN3YHCJTGT5FG45XWH5FGZRXL3A.data

LEAF1=bafkreibkrcw7hf6nhr6dvwecqxc5rqc7u7pkhkti53byyznqp23dk5fc2y
LEAF1FILE=.ipfs/blocks/C2/AFKREIBKRCW7HF6NHR6DVWECQXC5RQC7U7PKHKTI53BYYZNQP23DK5FC2Y.data

LEAF2=bafkreidfsuir43gjphndxxqa45gjvnrzbet3crpumyjcblk3rtn7zamq6q
LEAF2FILE=.ipfs/blocks/Q6/BAFKREIDFSUIR43GJPHNDXXQA45GJVNRZBET3CRPUMYJCBLK3RTN7ZAMQ6Q

LEAF3=bafkreihsipwnaj3mrc5plg24lpy6dw2bpixl2pe5iapzvc6ct2n33uhqjm
LEAF4=bafkreihrzs3rh4yxel4olv54vxettu5hv6wxy3krh6huzwhjub7kusnen4
test_expect_success "find data blocks for added file" '
HASH1MH=`cid-fmt -b base32 "%M" $HASH1` &&
LEAF1MH=`cid-fmt -b base32 "%M" $LEAF1` &&
LEAF2MH=`cid-fmt -b base32 "%M" $LEAF2` &&
HASH1FILE=`find .ipfs/blocks -type f | grep -i $HASH1MH` &&
LEAF1FILE=`find .ipfs/blocks -type f | grep -i $LEAF1MH` &&
LEAF2FILE=`find .ipfs/blocks -type f | grep -i $LEAF2MH`
'

test_expect_success "remove a leaf node from the repo manually" '
rm "$LEAF1FILE"
'

test_expect_success "check that the node is removed" '
test_must_fail ipfs cat $HASH1
'
test_expect_success "check that the node is removed" '
test_must_fail ipfs cat $HASH1
'

test_expect_success "'ipfs repo gc' should still be fine" '
ipfs repo gc
Expand Down Expand Up @@ -69,12 +73,14 @@ test_gc_robust_part1() {
grep -q "permission denied" block_rm_err
'

# repo gc outputs raw multihashes. We chech HASH1 with block stat rather than
# grepping the output since it's not a raw multihash
test_expect_success "'ipfs repo gc' should still run and remove as much as possible" '
test_must_fail ipfs repo gc 2>&1 | tee repo_gc_out &&
grep -q "removed $HASH1" repo_gc_out &&
grep -q "could not remove $LEAF2" repo_gc_out &&
grep -q "removed $LEAF3" repo_gc_out &&
grep -q "removed $LEAF4" repo_gc_out
grep -q "removed $(to_raw_cid $LEAF3)" repo_gc_out &&
grep -q "removed $(to_raw_cid $LEAF4)" repo_gc_out &&
Comment on lines +81 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

These don't seem necessary since we're already doing raw leaves, but it can't hurt

test_must_fail ipfs block stat $HASH1
'

test_expect_success "fix the permission problem" '
Expand All @@ -83,7 +89,7 @@ test_gc_robust_part1() {

test_expect_success "'ipfs repo gc' should be ok now" '
ipfs repo gc | tee repo_gc_out
grep -q "removed $LEAF2" repo_gc_out
grep -q "removed $(to_raw_cid $LEAF2)" repo_gc_out
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem necessary since we're already doing raw leaves, but it can't hurt

'
}

Expand All @@ -100,6 +106,7 @@ test_gc_robust_part2() {
LEAF2=QmTbPEyrA1JyGUHFvmtx1FNZVzdBreMv8Hc8jV9sBRWhNA
LEAF2FILE=.ipfs/blocks/WM/CIQE4EFIJN2SUTQYSKMKNG7VM75W3SXT6LWJCHJJ73UAWN73WCX3WMY.data


test_expect_success "add some additional unpinned content" '
random 1000 3 > junk1 &&
random 1000 4 > junk2 &&
Expand Down Expand Up @@ -147,8 +154,8 @@ test_gc_robust_part2() {

test_expect_success "'ipfs repo gc' should be fine now" '
ipfs repo gc | tee repo_gc_out &&
grep -q "removed $HASH2" repo_gc_out &&
grep -q "removed $LEAF2" repo_gc_out
grep -q "removed $(to_raw_cid $HASH2)" repo_gc_out &&
grep -q "removed $(to_raw_cid $LEAF2)" repo_gc_out
'
}

Expand Down
4 changes: 2 additions & 2 deletions test/sharness/t0275-cid-security.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ test_cat_get() {

test_gc() {
test_expect_success "injecting insecure block" '
mkdir -p "$IPFS_PATH/blocks/JZ" &&
cp -f ../t0275-cid-security-data/AFKSEBCGPUJZE.data "$IPFS_PATH/blocks/JZ"
mkdir -p "$IPFS_PATH/blocks/TS" &&
cp -f ../t0275-cid-security-data/EICEM7ITSI.data "$IPFS_PATH/blocks/TS"
'

test_expect_success "gc works" 'ipfs repo gc > gc_out'
Expand Down
6 changes: 3 additions & 3 deletions test/sharness/t0276-cidv0v1.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,19 @@ test_expect_success "check hashes" '
'

test_expect_success "make sure CIDv1 hash really is in the repo" '
ipfs refs local | grep -q $AHASHv1
ipfs block stat $AHASHv1
'

test_expect_success "make sure CIDv0 hash really is in the repo" '
ipfs refs local | grep -q $AHASHv0
ipfs block stat $AHASHv0
'

test_expect_success "run gc" '
ipfs repo gc
'

test_expect_success "make sure the CIDv0 hash is in the repo" '
ipfs refs local | grep -q $AHASHv0
ipfs block stat $AHASHv0
'

test_expect_success "make sure we can get CIDv0 added file" '
Expand Down
Loading