Remove X-chain UniqueTx #1662
Remove X-chain UniqueTx #1662
Conversation
StephenButtolph
added
vm
vms/avm/chain_state.go
Outdated
There was a problem hiding this comment.
This implementation is now used for all X-chain state.
vms/avm/tx.go
Outdated
| tx.vm.state.AddTx(tx.tx) | ||
|
|
||
| txID := tx.ID() | ||
| tx.vm.state.DeleteStatus(txID) |
There was a problem hiding this comment.
We assume that transactions on disk with no status are accepted, so we just need to make sure to delete any previously existing statuses on disk.
vms/avm/unique_tx.go
Outdated
vms/avm/vm.go
Outdated
| @@ -579,7 +557,7 @@ func (vm *VM) initState(tx *txs.Tx) { | |||
| zap.Stringer("txID", txID), | |||
| ) | |||
| vm.state.AddTx(tx) | |||
| vm.state.AddStatus(txID, choices.Accepted) | |||
| vm.state.DeleteStatus(txID) | |||
There was a problem hiding this comment.
We assume that transactions on disk with no status are accepted, so we just need to make sure to delete any previously existing statuses on disk.
Here that should never happen - but for clarity I think it makes sense to explicitly remove the status anyways.
| @@ -426,77 +419,6 @@ func TestVMFormat(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| func TestTxCached(t *testing.T) { | |||
There was a problem hiding this comment.
These were testing the UniqueTx caching behavior - which is now removed.
| firstTxStatus, err := env.vm.state.GetStatus(firstTx.ID()) | ||
| _, err = env.vm.state.GetTx(firstTx.ID()) | ||
| require.NoError(err) | ||
| require.Equal(choices.Accepted, firstTxStatus) | ||
|
|
||
| secondTxStatus, err := env.vm.state.GetStatus(secondTx.ID()) | ||
| _, err = env.vm.state.GetTx(secondTx.ID()) | ||
| require.NoError(err) | ||
| require.Equal(choices.Accepted, secondTxStatus) |
There was a problem hiding this comment.
GetTx only returns Accepted transactions now.
There was a problem hiding this comment.
I don't think that's documented at the moment:
type ReadOnlyChain interface {
avax.UTXOGetter
GetTx(txID ids.ID) (*txs.Tx, error)
GetBlockID(height uint64) (ids.ID, error)
GetBlock(blkID ids.ID) (blocks.Block, error)
GetLastAccepted() ids.ID
GetTimestamp() time.Time
}perhaps we should do that?
There was a problem hiding this comment.
I removed the DeleteStatus from the State interface. Now - the State interface returns txs that were AddTxed. The State interface no longer references statuses at all.
vms/avm/states/state.go
Outdated
| // | ||
| // TODO: Remove DeleteStatus after v1.11.x has activated and we can assume | ||
| // all nodes have pruned their statuses from disk. | ||
| DeleteStatus(id ids.ID) |
There was a problem hiding this comment.
If we can assume all nodes have pruned their statuses from disk after v1.1.11.x has activated, why don't we just remove entire statusDB and statusCache instead?
There was a problem hiding this comment.
After v1.11.x we will remove the statusDB and statusCache
vms/avm/state_test.go
Outdated
| @@ -75,18 +74,15 @@ func TestSetsAndGets(t *testing.T) { | |||
|
|
|||
| env.vm.state.AddUTXO(utxo) | |||
| env.vm.state.AddTx(tx) | |||
| env.vm.state.AddStatus(txID, choices.Accepted) | |||
| env.vm.state.DeleteStatus(txID) | |||
There was a problem hiding this comment.
Can we document why we do this? It's not super obvious what we're doing here, and the test passes without this line. OTOH we're removing DeleteStatus soon so maybe we don't need to?
There was a problem hiding this comment.
It isn't needed - I felt it was most clear to always delete the status when accepting a tx to make it clear that the status will be reported correctly.
This is the same as deleting the status in the genesis initialization.
There was a problem hiding this comment.
I removed the DeleteStatus function entirely.
danlaine
force-pushed
the
remove-statuses
branch
from
fe7a241 to
0f8366e
Compare
|
I'm going to merge this - the mainnet sync is still ongoing but is expected to pass. |
Why this should be merged
UniqueTx is no longer needed since all AVM instances have been linearized.
How this works
ParseTx.How this was tested