[Merged by Bors] - Allow vaults to spend received funds

[lrettig]

Motivation

Fixes an oversight in genvm implementation: vesting vaults are unable to spend received coins.

Description

Adds a Balance() method to host/context to allow genvm templates to read current account balance, then updates vault spending arithmetic accordingly.

Test Plan

Added tests in genvm/templates/vault/vault_test.go

TODO

• Explain motivation or link existing issue(s)
• Test changes and document test plan
• Update documentation as needed
• Update changelog as needed

[codecov]

Codecov Report

Attention: Patch coverage is 92.85714% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 80.4%. Comparing base (3e9b225) to head (9a26373).

Files	Patch %	Lines
genvm/templates/vault/vault.go	90.0%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           develop   #5840   +/-   ##
=======================================
  Coverage     80.4%   80.4%           
=======================================
  Files          285     285           
  Lines        29510   29514    +4     
=======================================
+ Hits         23750   23756    +6     
+ Misses        4161    4159    -2     
  Partials      1599    1599           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lrettig]

thanks @fasmat. let's get a second review on this one since the code path is pretty critical.

[lrettig]

@dshulyak pointed out that there's nothing currently preventing a misconfigured vault (with, say, an actual balance less than the TotalAmount) from being spawned. I addressed this, please see the latest commits.

This however raises the question - do we want to allow Spawning of misconfigured/invalid Vaults in the first place? Examples would be an insufficient balance, or VestingStart > VestingEnd. With the code as it stands there's no harm in allowing them, and I've written the logic here in such a way that the funds in the vault can still be spent. We may also want to prevent Spawn of any Vault other than the genesis Vaults to prevent this sort of issue.

If VestingStart > VestingEnd, no Spend can occur until VestingStart layer, at which point the TotalAmount becomes available.
If account balance < unvested portion of TotalAmount, no Spend can occur, but this can always be remedied by sending more coins to the account.

[poszu]

This however raises the question - do we want to allow Spawning of misconfigured/invalid Vaults in the first place? Examples would be an insufficient balance, or VestingStart > VestingEnd. With the code as it stands there's no harm in allowing them, and I've written the logic here in such a way that the funds in the vault can still be spent. We may also want to prevent Spawn of any Vault other than the genesis Vaults to prevent this sort of issue.

I prefer when objects in invalid state cannot exist in the first place rather than have to have checks around in the logic because the checks are easy to miss or get wrong.

If VestingStart > VestingEnd, no Spend can occur until VestingStart layer, at which point the TotalAmount becomes available. If account balance < unvested portion of TotalAmount, no Spend can occur, but this can always be remedied by sending more coins to the account.

I think VestingStart > VestingEnd is not possible because of this check:

go-spacemesh/genvm/templates/vault/handler.go

Lines 42 to 45 in e2c579a

	if spawn.VestingEnd.Before(spawn.VestingStart) {
	return nil, fmt.Errorf("vesting end %s should be atleast equal to start %s",
	spawn.VestingEnd, spawn.VestingStart)
	}

However, VestingStart == VestingEnd is possible, in which case the code in

go-spacemesh/genvm/templates/vault/vault.go

Line 49 in e2c579a

vested.Div(vested, new(big.Int).SetUint64(uint64(v.VestingEnd.Difference(v.VestingStart))))

would crash (division by zero).

[lrettig]

I prefer when objects in invalid state cannot exist in the first place rather than have to have checks around in the logic because the checks are easy to miss or get wrong.

Makes sense. But the question is orthogonal to the fixes in this PR. Let's get this merged and then address that question separately.

However, VestingStart == VestingEnd is possible, in which case the code in

go-spacemesh/genvm/templates/vault/vault.go

Line 49 in e2c579a

vested.Div(vested, new(big.Int).SetUint64(uint64(v.VestingEnd.Difference(v.VestingStart))))

would crash (division by zero).

Actually division by zero is impossible here. If VestingStart == VestingEnd then one of these two conditionals will be triggered first:

go-spacemesh/genvm/templates/vault/vault.go

Lines 41 to 46 in e2c579a

	if lid.Before(v.VestingStart) {
	return 0
	}
	if !lid.Before(v.VestingEnd) {
	return v.TotalAmount
	}

Note that there's already a test for this:

go-spacemesh/genvm/templates/vault/vault_test.go

Lines 304 to 312 in e2c579a

	{
	desc: "vest start equals end",
	start: 2,
	end: 2,
	lid: 1,
	total: 1000,
	spend: 1,
	expect: ErrAmountNotAvailable,
	},

I'll add a comment in the code to this effect.

[lrettig]

bors merge

[spacemesh-bors]

Build failed:

• ci-status

[lrettig]

bors merge

[spacemesh-bors]

Build failed:

• ci-status

[lrettig]

bors merge

[spacemesh-bors]

Pull request successfully merged into develop.

Build succeeded:

• ci-status
• systest-status