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.

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

storage incentives #1562

Merged
merged 27 commits into from
Apr 27, 2021
Merged

storage incentives #1562

merged 27 commits into from
Apr 27, 2021

Conversation

acud
Copy link
Member

@acud acud commented Apr 13, 2021 •
edited
Loading

supersedes #1551

Todos:

  • OpenAPI spec
  • allow data import with a batch ID that will stamp all imported chunks
  • indexing scheme
  • cli flag for capacity turns into cache size (note: let's do this AFTER merge to master, otherwise we can't do integration testing till then as the helm charts would be broken) Change db-capacity flag to cache-capacity #1372
  • bee-repair export existing database
  • bee add cli command to export existing database
  • hook up kademlia depth upper bound with the reserve radius
  • add postage batch id to api endpoints
  • double check no fallback batch ids anywhere
  • improve bucket collisions depth in-memory representation / limit the size not needed for now as this is not prohibitive in terms of memory and @zelig has added an upper bound of how many buckets we keep track of in-mem. as a later optimization we can try to find a cheaper way to track this.
  • increment export version in localstore export.go
  • listener needs to return a channel that will be closed once it is done syncing (paging)

@acud acud mentioned this pull request Apr 13, 2021
Closed
4 tasks
@acud acud force-pushed the storage-incentives-rebased branch from 0964a01 to a6c986c Compare April 15, 2021 11:31
acud
acud commented Apr 26, 2021
View reviewed changes
pkg/api/api.go Outdated Show resolved Hide resolved
acud
acud commented Apr 26, 2021
View reviewed changes
pkg/api/api_test.go Outdated Show resolved Hide resolved
@acud acud force-pushed the storage-incentives-rebased branch 2 times, most recently from 227a3c4 to 098d553 Compare April 26, 2021 07:40
acud
acud commented Apr 26, 2021
View reviewed changes
pkg/debugapi/debugapi_test.go Outdated
@@ -162,7 +164,7 @@ func TestServer_Configure(t *testing.T) {
}),
)

s.Configure(o.P2P, o.Pingpong, topologyDriver, o.Storer, o.Tags, acc, settlement, true, swapserv, chequebook)
s.Configure(o.P2P, o.Pingpong, topologyDriver, o.Storer, o.Tags, acc, settlement, true, swapserv, chequebook, nil)
Copy link
Member Author

@acud acud Apr 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

needs batchstore here, test should also reflect this change

acud
acud commented Apr 26, 2021
View reviewed changes
pkg/postage/batchstore/reserve.go Show resolved Hide resolved
pkg/postage/batchstore/reserve.go Show resolved Hide resolved
pkg/postage/batchstore/store.go Outdated Show resolved Hide resolved
pkg/postage/batchstore/store.go Show resolved Hide resolved
pkg/postage/batchstore/store.go Outdated Show resolved Hide resolved
pkg/postage/listener/listener.go Show resolved Hide resolved
pkg/postage/listener/listener.go Show resolved Hide resolved
pkg/postage/listener/listener.go Outdated Show resolved Hide resolved
pkg/postage/listener/listener.go Outdated Show resolved Hide resolved
pkg/postage/listener/listener_test.go Show resolved Hide resolved
zelig
zelig reviewed Apr 26, 2021
View reviewed changes
Copy link
Member

@zelig zelig left a comment

Choose a reason for hiding this comment

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

upto localstore mode_set

pkg/api/soc_test.go Outdated Show resolved Hide resolved
pkg/localstore/mode_get.go Outdated Show resolved Hide resolved
pkg/localstore/mode_set.go Show resolved Hide resolved
pkg/localstore/mode_set.go Show resolved Hide resolved
@acud acud force-pushed the storage-incentives-rebased branch from 3a080a1 to e482476 Compare April 27, 2021 05:00
zelig
zelig reviewed Apr 27, 2021
View reviewed changes
pkg/localstore/reserve_test.go Outdated

t.Run("postage chunks index count", newItemsCountTest(db.postageChunksIndex, int(gcTarget)))

// postageRadiusIndex "leaks" since the batches of the evicted chunks were not called with
Copy link
Member

Choose a reason for hiding this comment

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

this issue should be resolves so that no such comment is needed, pls double check

Copy link
Member Author

Choose a reason for hiding this comment

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

no this comment is correct 100%. the index leaks until all associated chunks get evicted

acud and others added 14 commits April 27, 2021 11:16
@acud
api: accept postage id header and propagate it to pipeline (#886)
97835f3 
postage: new pkg for postage stamps, uploader stamping (#890)

* postage: new pkg for postage stamps, uploader stamping

* postage: amount->value, blockNumber big.Int-> uint64, stamp only has batch ID, not Batch

* postage: fix godoc and copyright

* postage, swarm:
 - swarm.Stamp as an interface
 - add postage/testing for mock Stamps
 - fix Stamp MarshalBinary to allow nil batch id and signature
 - add StampSize const

* postage: heed review feedback

Co-authored-by: acud <[email protected]>

Storage incentives: add stamper putter to api

Postage BatchStore and BatchService (#1070)

Co-authored-by: zelig <[email protected]>

add normalisedBalance to updater interface (#1108)

make value the normalised balance (#1111)

postage: add event listener (#1099)

Wire up postage stamp syncing (#1114)

localstore, shed: persist stamps (#1116)

add --postage to beeinfra.sh setup

pullsync, pushsync: add postage stamps (#1117)

postage: add create endpoint (#1142)

retrieve erc20 address from postage contract (#1169)

postage: check balance before attempting stamp creation (#1177)

postage: fix bucket depth (#1178)

api: use hex encoding in postage api (#1179)

increase page size (#1182)

postage: handle bucket depth error in api (#1183)

localstore: attach stamp to outgoing chunk (#1192)

update postage stamp contract addresses for new token (#1208)

batchstore: reserve (#1262)

* postage/batchstore: reserve logic

Co-authored-by: acud <[email protected]>

stamp support in storage and protocols (#1321)

api: endpoints for stamp issuers (#1535)

retrieval: add stamps (#1552)

localstore reserve logic (#1322)

Co-authored-by: acud <[email protected]>
@istae @acud
chunks: batchID (#1572)
3b1627d 
* chunks: batchID
@acud
batch service: improve logging (#1574)
493bbf9
@istae @acud
postage listender polling time reduced to 5s (#1575)
a2863fb
@acud
api: stamper propagation (#1569)
ddcf544
@acud
add metrics (#1588)
e0ef457
@acud
localstore: increment export version (#1584)
aad1a5f
@istae @acud
reserve state debug api (#1586)
cf0a1f1
@acud
batchstore, kademlia: add reserve radius as depth upper bound (#1594)
a469671
@acud
storage incentives: reduce deepsource complaints (#1596)
7700903
@acud
batchstore: fix panic (#1598)
70deeff
@ralph-pichler @acud
use abi library (#1599)
6a49005
@acud
storage-incentives: cleanups (#1602)
09dad20 
* deprecate empty batches fallback
* adjust bucketDepth and default radius
* fix long standing rebase error in node.go some renaming improvments too
@acud
node: wait till synced with postage contract before starting node (#1603
f30d68d 
)
@acud acud force-pushed the storage-incentives-rebased branch from b09eebf to 415bb0a Compare April 27, 2021 09:20
ralph-pichler
ralph-pichler approved these changes Apr 27, 2021
View reviewed changes
Copy link
Member

@ralph-pichler ralph-pichler left a comment

Choose a reason for hiding this comment

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

looks good. the non-determinism issue in the listener needs to be fixed before this hits production, but that can be done after this is merged.

pkg/postage/listener/listener.go
// so that the eviction in batchstore gets the correct
// block height context for the gc round. otherwise
// expired batches might be "revived".
err = updater.UpdateBlockNumber(to)
Copy link
Member

Choose a reason for hiding this comment

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

this makes syncing the reserve state a potentially non-deterministic process as depending on wether there is paging there is an increase in Total by 10000 blocks of price in advance which could lead to a batch being evicted even though there is a valid top up within that range which would not happen if the blocks were processed individually (which would be the case if the bee node was online while these blocks came in one-by-one).

pkg/postage/listener/listener.go
closeOnce.Do(func() { close(synced) })
}

events, err := l.ev.FilterLogs(ctx, l.filterQuery(big.NewInt(int64(from)), big.NewInt(int64(to))))
Copy link
Member

Choose a reason for hiding this comment

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

this might break with certain endpoints if there are too many (postage) events in that range.

ralph-pichler
ralph-pichler reviewed Apr 27, 2021
View reviewed changes
pkg/node/node.go
}
b.ethClientCloser = swapBackend.Close
b.transactionMonitorCloser = transactionMonitor
swapBackend, overlayEthAddress, chainID, transactionMonitor, transactionService, err := InitChain(
Copy link
Member

Choose a reason for hiding this comment

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

maybe this should be in an if !o.Standalone, otherwise standalone mode will require an eth backend.

@acud acud merged commit 84f54d7 into master Apr 27, 2021
@Eknir Eknir mentioned this pull request May 20, 2021
Closed
@vladopajic vladopajic deleted the storage-incentives-rebased branch March 21, 2023 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ralph-pichler ralph-pichler ralph-pichler approved these changes

@zelig zelig zelig approved these changes

Assignees
No one assigned
Labels
pull-request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@acud @zelig @ralph-pichler @istae