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

add config options for batching deal publish #658

Merged
merged 3 commits into from
Feb 6, 2021
Merged

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented Feb 4, 2021

No description provided.

@jennijuju
Copy link
Member

jennijuju commented Feb 4, 2021

@johnnymatthews could you please merge this when v1.4.2 is released? scheduled next week

@f8-ptrk
Copy link
Contributor

f8-ptrk commented Feb 4, 2021

@dirkmc the 1h (in the example) applies to the first or the last message added to the "batch"

msg1: 7:30 ---> "batch wait" until 8:30
msg2: 7:45 ---> "batch wait" until ???

is there a hard max. on MaxDealsPerPublishMsg i assume we are at least limited by how much gas a block can use? i saw 30 deals batched on chain - but no idea if that's the limit or not

@dirkmc
Copy link
Contributor Author

dirkmc commented Feb 4, 2021

@PatrickDeuse I added some more explanation. Is it clear now?

I don't believe there is any limit on MaxDealsPerPublishMsg, but you probably don't want to include too many deals as if one deal fails validation, all deals included in the batched publish message will fail: filecoin-project/specs-actors#1375

@f8-ptrk
Copy link
Contributor

f8-ptrk commented Feb 4, 2021

@dirkmc crystal clear.

there must be a natural limit on how many deals we can put inside to get the message on chain. be it a maxInt we hit somewhere...

we will figure it out as soon as the calibnet is back on the battlefield

@ribasushi
Copy link
Contributor

ribasushi commented Feb 4, 2021

@dirkmc how does this interface with palcement-into-sectors: that is when does the sector/piece assignment happen:

  • when the batch-publish completes
    or
  • does it happen on deal-acceptance?

( don't have a working miner to check this locally )

@f8-ptrk
Copy link
Contributor

f8-ptrk commented Feb 4, 2021

@ribasushi the placement into sectors is done after the publishdeals message hits the chain.

technically "when the batch-publish completes" and "deal-acceptance" are the same thing! see my, kind of, "don't touch my money" rant here:

filecoin-project/lotus#5444

someone might want to come up with a different wording for the offchain logic of the deals that doesn't contradict what the on chain logic implies, when a deal is "Accepted" in particular

@jennijuju jennijuju merged commit 1f1ee8e into master Feb 6, 2021
@jennijuju jennijuju deleted the feat/batch-publish branch February 6, 2021 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants